FsVolume support partition? Also maybe memory leaks

[KurtE]

I have been playing some trying to make a few contributions to the Teensy versions of this in particular most recently playing with trying to integrate MTP and MSC stuff...

While doing this, I have run into a few things that might be issues and/or maybe nice enhancements.

Both FatVolume and ExFatVolume classes support partitions, but the FsVolume does not. In the simplest case it might be easy as: as adding the two optional parameters to begin, like the other two classes:

  bool begin(BlockDevice* blockDev, bool setCwv = true, uint8_t part = 1);

and update the implementation to pass these through:

bool FsVolume::begin(BlockDevice* blockDev, bool setCwv, uint8_t part) {
  Serial.printf("FsVolume::begin(%x)\n", (uint32_t)blockDev);
  m_blockDev = blockDev;
  m_fVol = nullptr;
  m_xVol = new (m_volMem) ExFatVolume;
  if (m_xVol && m_xVol->begin(m_blockDev, setCwv, part)) {
    goto done;
  }
  m_xVol = nullptr;
  m_fVol = new (m_volMem) FatVolume;
  if (m_fVol && m_fVol->begin(m_blockDev, setCwv, part)) {
    goto done;
  }
  m_cwv = nullptr;
  m_fVol = nullptr;
  return false;

 done:
  m_cwv = this;
  return true;
}

Warning my modern C++ code stuff is a bit rusty, but I think this class is leaking the FatVolume and the ExFatVolume objects.

That is both in the end method as well as the begin: maybe the end should be changed from:

  void end() {
    m_fVol = nullptr;
    m_xVol = nullptr;
  }

to something like:

  void end() {
    if (m_fVol) {
      delete m_fVol;
      m_fVol = nullptr;
    }
    if (m_xVol ) {
      delete m_xVol ;
      m_xVol = nullptr;
   } 
 }

And likewise in the begin method if the exFatVolume begin returned false, should probably delete it before setting to nullptr.

[greiman]

No I don't really use dynamic memory. For embedded systems in my world it was forbidden. SdFat never uses the heap.

I define my own new operator to share RAM between FAT and exFAT since only one can be active.

https://en.cppreference.com/w/cpp/memory/new/operator_new

Here is the magic: FsNew.h

/** 32-bit alignment */
typedef uint32_t newalign_t;

/** Custom new placement operator */
void* operator new(size_t size, newalign_t* ptr);

FsNew.cpp

#include "FsNew.h"
void* operator new(size_t size, newalign_t* ptr) {
  (void)size;
  return ptr;
}

It's allocated in FsVolume.h here. FS_ALIGN_DIM is the max size required. newalign_t m_volMem[FS_ALIGN_DIM(ExFatVolume, FatVolume)];

[greiman]

The SD standard does not support partitions so SdFat begin does not allow partitions. I wrote the library to support partitions on devices that support partitions.

The SD association specifies the layout of the files system including hidden sectors, cluster size etc. The structure of a card's flash, buffers, and algorithms depends on not just one partition but the location of file system structures and cluster sizes.

You should never use an OS utility to reformat an SD card. Use the SD Association formatter .

Here is a quote from the SD Association.

This specification specifies SD file systems. The type of the file system to be used shall be uniquely decided with Card Capacity as follows. Here, Card Capacity means the total size of Data Area and Protected Area size.

File System Type That is, all Standard Capacity SD Memory Cards whose capacity is 2048MB or less shall use FAT12 / FAT16 file system, and never use the other file system. Similarly, all High Capacity SD Memory Cards shall use FAT32 file system, and never use the other file system. And all Extended Capacity SD Memory Cards shall use exFAT file system, and never use the other file system. This includes the prohibition of partial format of SD Memory Card. For example, 8GB High Capacity SD Memory Card should not be formatted as 2GB card with FAT12 / FAT16 file system. In this case, whole area of 8GB should be formatted with FAT32 file system.

[KurtE]

Thanks,

Makes sense on the memory. I was sort of surprised if you were actually allocating memory.

As for Partitions on SD Cards, Maybe the SD Association never saw an SDCard used in a Raspberry Pi ;)

And it is interesting that each platform has different File systems they wish to format as...

Must be fun trying to support as many as you can.

Thanks again

[greiman]

The next Raspberry Pi should support a real M.2 SSD, small cards are cheap.

Most SD card work with non-standard formats but the internals of the card are designed for the standard format.

On a FAT card there are small caches for the two FAT areas. The data area has huge caches and flash pages aligned with file structures. A "Record Unit" on a modern card is 512 KB.

For reliability is assumed that some critical areas will never be written these areas won't need to be remapped due to wear leveling.

[KurtE]

Yep with some of the different Micro Controllers like RPI and Odroid boards, I usually tried to avoid the SD Cards...

But now back to the sort of issue at hand:

With the MSC work going on with the Teensy 3.6 or 4.x, we are working on supporting things like thumb drives and other USB block devices that one might plug in, and also with it am also working on allowing it to integrate in with MTP and as such those drives might be visible to the PC...

The partition code currently in this library works with the FatVolume and ExFatVolume and for example we have a kludged up example sketch up in the MSC procject (https://github.com/wwatson4506/UsbMscFat/blob/main/examples/VolumeName/VolumeName.ino) that for example with one of my USB Thumb drives I have 3 partitions, on Fat16, another Fat32 and another ExFat, and it sees there are the three partitions, can call the appropriate ls method, and we have a set of code that can get the Volume label for the partitions...

Now I would be great to clean up this code, to make some of it reasonable then use with the MTP code, where for example the Storage name you pass to the host, may include the Volume Label for the storage...

Obviously we can do our own wrapping code/class to say is it an ExFat volume, then call it, else call Fat function, but that more or less is replicating the same code as the FsVolume...

Another option would be to have the FsLib volume code allow access to the two logical pointers, (either like make it protected: instead of private, then one could subclass it to allow the subclass to have a begin method with which partition...

Code wise, with the stuff from the original message here: Really not much of a change to add the two optional parameters, like the FatLib and ExFatLib have and pass them through, i.e. instead of:

if (m_xVol && m_xVol->begin(m_blockDev, false)) {

if (m_xVol && m_xVol->begin(m_blockDev, setCwv, part)) {

But for now I will probably just as easy for me duplicate the code.

[greiman]

Adafruit, STM32duino, ESP8266 and others are making major change to their clone of SdFat. You can do what ever you want with Teensy.

I will assume that I should direct users to the Teensy version and will no longer test my mods on Teensy just as I don't test on Adafruit, STM32, or ESP8266 board packages.

[KurtE]

Understand,

Not sure if there is/will be an official Teensy version. Right now I am experimenting and prefer in the end to continue to work with the official version, unless there is a strong reason not to. That is if I clone it will only be the FsLib part of the code, which would still continue to use the ExFatlib and FatLib... And as I mentioned was seeing your thoughts on simply allowing the partition setting in the begin to pass through to your other two sub-libraries.

And likewise if any of the other additions that you are interested in and/or have plans to do...

Things like, if I clone the FsLib will probably add in a new method to get the volume ID. As I can easily imagine others might be interested in knowing if the user put in the volume that they think they did...

Also if you from another Issue/PR I remember seeing a request to add the ability to get the create and modify dates from file. Currently you do optionally display this data in the ls method. And I believe one of your comments was you would be adding this in the next release... Are you still planning on this?

Again I like to contribute to stuff when ever I can.

[greiman]

Here are get set date functions:

 /** Get a file's access date and time.
   *
   * \param[out] pdate Packed date for directory entry.
   * \param[out] ptime Packed time for directory entry.
   *
   * \return true for success or false for failure.
   */
  bool getAccessDateTime(uint16_t* pdate, uint16_t* ptime);
  /** Get a file's create date and time.
   *
   * \param[out] pdate Packed date for directory entry.
   * \param[out] ptime Packed time for directory entry.
   *
   * \return true for success or false for failure.
   */
  bool getCreateDateTime(uint16_t* pdate, uint16_t* ptime);
  /** Get a file's Modify date and time.
   *
   * \param[out] pdate Packed date for directory entry.
   * \param[out] ptime Packed time for directory entry.
   *
   * \return true for success or false for failure.
   */
  bool getModifyDateTime(uint16_t* pdate, uint16_t* ptime);
  /** Set a file's timestamps in its directory entry.
   *
   * \param[in] flags Values for \a flags are constructed by a bitwise-inclusive
   * OR of flags from the following list
   *
   * T_ACCESS - Set the file's last access date and time.
   *
   * T_CREATE - Set the file's creation date and time.
   *
   * T_WRITE - Set the file's last write/modification date and time.
   *
   * \param[in] year Valid range 1980 - 2107 inclusive.
   *
   * \param[in] month Valid range 1 - 12 inclusive.
   *
   * \param[in] day Valid range 1 - 31 inclusive.
   *
   * \param[in] hour Valid range 0 - 23 inclusive.
   *
   * \param[in] minute Valid range 0 - 59 inclusive.
   *
   * \param[in] second Valid range 0 - 59 inclusive
   *
   * \note It is possible to set an invalid date since there is no check for
   * the number of days in a month.
   *
   * \note
   * Modify and access timestamps may be overwritten if a date time callback
   * function has been set by dateTimeCallback().
   *
   * \return true for success or false for failure.
   */
  bool timestamp(uint8_t flags, uint16_t year, uint8_t month, uint8_t day,
                 uint8_t hour, uint8_t minute, uint8_t second);

My plan for SdFat is to only fix bugs for the Arduino version of SdFat for sometime, probably months.

I am now spending my time playing with VxWorks 7. In the early 1980s I was involved with early development of VxWorks, the RTOS used in the SpaceX rocket and Dragon capsule and the recent the Mars lander.

Did you see the video of the crew using touch screens during the Dragon launch?

I am running it on a Pi 4. It is astounding, there are about 2,000 board support packages. The stripped down Pi 4 version is 7GB.

[KurtE]

Thanks,

Cool stuff.