- fix several memory leaks

locatw · locatw · commit ef496059019c · 2024-04-10T17:19:58.000+02:00
- rework member variables when calling begin/umount/format (solves inconsistenies).
  begin() means mounting. It must not initialize the block device, as this is global to a partition.
  format() is only possbile when FS is not mounted (it makes no sense to call begin()/unmount(),
  just to get a valid mbr block device).
  -&gt; rework
- When creating partition object and no partition was present on block device, the members of partition object
  left uninitialzed.
- add function to free the allocated memory that was returned when calling utils function to create a partition name.
- solve problems, where block devices keept initialized when calling listPartitions().
  This function could only be called two times. Other called functions failed because
  of the blocked resources. It now calls deinit() to leave it clean.
diff --git a/examples/InternalStoragePartitioning/InternalStoragePartitioning.ino b/examples/InternalStoragePartitioning/InternalStoragePartitioning.ino
@@ -4,13 +4,13 @@
     This example demonstrates the usage of the "Arduino_UnifiedStorage" library for retrieving and creating partitions on the internal storage.
     The code should help you understand how to work with partitions and perform file operations in different partitions.
 
-    It creates the partitions specified in the std::vector<Partitions> you find at the top of the sketch. 
+    It creates the partitions specified in the std::vector<Partitions> you find at the top of the sketch.
     You can define your own, as long as the size of all partitions doesn't exceed the size of your board's QSPI flash( if you are in doubt about that check docs.arduino.com for more information) and as long as you don't have more than 4 partitions (MBR limitation)
     The Partition struct has two values:
         - `size` the size of your partition in kilobytes
         - 'fileSystemType` which can be either `FS_FAT` or `FS_LITTLEFS`
 
-    Here are a few examples of valid partitioning schemes: 
+    Here are a few examples of valid partitioning schemes:
         - std::vector<Partition> partitioningScheme  = {{16384, FS_FAT}};
         - std::vector<Partition> partitioningScheme  = {{2048, FS_FAT}, {6144, FS_FAT} {8192, FS_LITTLEFS}};
         - std::vector<Partition> partitioningScheme  = {{4096, FS_LITTLEFS}, {4096, FS_FAT}, {4096, FS_LITTLEFS}, {4096, FS_FAT}};
@@ -24,8 +24,8 @@
     INSTRUCTIONS:
     1. Check compatibility with your board and make sure you have "POSIXStorage" and "Arduino_UnifiedStorage" installed
     2. Connect your board to the serial monitor
-    3. Wait for the sketch to run 
-    4. Modify the partitioning scheme according to your needs 
+    3. Wait for the sketch to run
+    4. Modify the partitioning scheme according to your needs
 
     Created: 26th October 2023
     By: Cristian Dragomir
@@ -36,7 +36,7 @@
 #include <vector>
 
 // Create a vector of partitions with one partition of 16MB using LittleFS
-std::vector<Partition> partitioningScheme  = {   
+std::vector<Partition> partitioningScheme  = {
         {1024, FS_FAT}, // 1 MB for certificates
         {5120, FS_FAT}, // 5 MB for OTA firmware updates
         {8192, FS_LITTLEFS} // 8 MB for user data
@@ -50,7 +50,7 @@ void testWriting(Arduino_UnifiedStorage *storage) {
     // Create a new file named "file.txt" for writing
     UFile file = root.createFile("file.txt", FileMode::WRITE);
     Serial.println("\t\t - File path: " + file.getPathAsString());
-    
+
     // Write data to the file
     file.write("writing stuff to the file");
 
@@ -65,7 +65,7 @@ void testWriting(Arduino_UnifiedStorage *storage) {
 void testAllPartitions(std::vector<Partition> partitions) {
     for (size_t i = 1; i < partitions.size() + 1; ++i) {
         const char *partitionName = createPartitionName(i);
-        
+
         // Create an InternalStorage object for the partition
         InternalStorage thisPartition = InternalStorage(i, partitionName, partitions[i - 1].fileSystemType);
 
@@ -74,6 +74,8 @@ void testAllPartitions(std::vector<Partition> partitions) {
             Serial.println("\t - Successfully mounted partition: /" + String(partitionName));
             Serial.println("\t - Testing file operations: ");
             testWriting(&thisPartition); // Test writing to a file in the partition
+            thisPartition.unmount();
+            freePartitionName(partitionName);
         }
 
         Serial.println();
@@ -113,7 +115,7 @@ void setup() {
 
     delay(1000);
 
-    // Read the MBR sector and display the partitions 
+    // Read the MBR sector and display the partitions
     listPartitions();
 }
 
diff --git a/library.properties b/library.properties
@@ -1,5 +1,5 @@
 name=Arduino_UnifiedStorage
-version=1.1.1
+version=1.1.2
 author=Arduino
 maintainer=Arduino <info@arduino.cc>
 sentence=Simplify cross-device storage management on Portenta platforms with a single library supporting SD, Flash, and USB storage access.
diff --git a/src/InternalStorage.cpp b/src/InternalStorage.cpp
@@ -2,25 +2,37 @@
 
 InternalStorage::InternalStorage(){
     std::vector<Partition> partitionsAvailable = Partitioning::readPartitions(QSPIFBlockDeviceType::get_default_instance());
-    if(partitionsAvailable.size() == 0){
+    if(partitionsAvailable.size() == 0)
+    {
 
         //Arduino_UnifiedStorage::debugPrint("[InternalStorage][INFO] No partitions found, restoring default partitions");
         restoreDefaultPartitions();
-    } else {
-        int lastPartitionNumber = partitionsAvailable.size();
-        FileSystems lastPartitionFileSystem = partitionsAvailable.back().fileSystemType;
-        //Arduino_UnifiedStorage::debugPrint("[InternalStorage][INFO] Found " + String(lastPartitionNumber) + " partitions, using last partition as internal storage");
-
-        this -> partitionNumber = lastPartitionNumber;
-        this -> fileSystemType = lastPartitionFileSystem;
-        this -> partitionName = (char *)"internal"; 
+        // re-read table
+        partitionsAvailable = Partitioning::readPartitions(QSPIFBlockDeviceType::get_default_instance());
     }
+
+    int lastPartitionNumber = partitionsAvailable.size();
+    FileSystems lastPartitionFileSystem = partitionsAvailable.back().fileSystemType;
+    //Arduino_UnifiedStorage::debugPrint("[InternalStorage][INFO] Found " + String(lastPartitionNumber) + " partitions, using last partition as internal storage");
+
+    this -> partitionNumber = lastPartitionNumber;
+    this -> fileSystemType = lastPartitionFileSystem;
+    this -> partitionName = (char *)"internal";
+    this -> blockDevice = BlockDeviceType::get_default_instance();
+    this -> mbrBlockDevice = new MBRBlockDeviceType(this -> blockDevice, this->partitionNumber);
 }
 
 InternalStorage::InternalStorage(int partition, const char * name, FileSystems fileSystemType){
     this -> partitionNumber = partition;
-    this -> partitionName = (char *)name;
+    this -> partitionName = name;
     this -> fileSystemType = fileSystemType;
+    this -> blockDevice = BlockDeviceType::get_default_instance();
+    this -> mbrBlockDevice = new MBRBlockDeviceType(this -> blockDevice, this->partitionNumber);
+}
+
+InternalStorage::~InternalStorage()
+{
+  delete this -> mbrBlockDevice;
 }
 
 bool InternalStorage::begin(FileSystems fileSystemType){
@@ -49,9 +61,6 @@ std::vector<Partition> InternalStorage::readPartitions(){
 }
 
 bool InternalStorage::begin(){
- 
-        this -> blockDevice = BlockDeviceType::get_default_instance();
-        this -> mbrBlockDevice = new MBRBlockDeviceType(this->blockDevice, this->partitionNumber);
 
         if(this -> fileSystemType == FS_FAT){
             this -> fileSystem = new FATFileSystemType(this->partitionName);
@@ -61,15 +70,23 @@ bool InternalStorage::begin(){
             Arduino_UnifiedStorage::debugPrint("[InternalStorage][begin][INFO] Mounting partition " + String(this->partitionNumber) + " as LittleFS");
         }
 
-        int err = this -> fileSystem -> mount(mbrBlockDevice);
+        int err = this -> fileSystem -> mount(this -> mbrBlockDevice);
         if(err!=0){
             Arduino_UnifiedStorage::debugPrint("[InternalStorage][ERROR] Could not mount partition " + String(this->partitionNumber) + " as " + prettyPrintFileSystemType(this->fileSystemType) + ", error code: " + String(errno));
         }
         return err == 0;
 }
 
 bool InternalStorage::unmount(){
-    int err = this -> fileSystem -> unmount();
+    int err = 0;
+
+    if(this -> fileSystem)
+    {
+      err = this -> fileSystem -> unmount();
+      delete this -> fileSystem;
+      this -> fileSystem = NULL;
+    }
+
     return err == 0;
 }
 
@@ -78,23 +95,25 @@ Folder InternalStorage::getRootFolder(){
 }
 
 bool InternalStorage::format(FileSystems fs){
-    this -> begin();
+    FileSystemType * tmpFileSystem = nullptr;
     this -> unmount();
     this -> fileSystemType = fs;
 
     if(fs == FS_FAT){
-            this -> fileSystem = new FATFileSystemType(this->partitionName);
-            int err = this -> fileSystem -> reformat(this-> mbrBlockDevice);
+            tmpFileSystem = new FATFileSystemType(this->partitionName);
+            int err = tmpFileSystem -> reformat(this-> mbrBlockDevice);
             if(err != 0){
                 Arduino_UnifiedStorage::debugPrint("[InternalStorage][format][ERROR] Error formatting partition " + String(this->partitionNumber) + " as FAT: " + String(errno));
-            } 
+            }
+            delete tmpFileSystem;
             return err == 0;
     } if (fs == FS_LITTLEFS) {
-            this -> fileSystem =  new LittleFileSystemType(this->partitionName);
-            int err = this -> fileSystem -> reformat(this-> mbrBlockDevice);
+            tmpFileSystem =  new LittleFileSystemType(this->partitionName);
+            int err = tmpFileSystem -> reformat(this-> mbrBlockDevice);
             if(err != 0){
                 Arduino_UnifiedStorage::debugPrint("[InternalStorage][format][ERROR] Error formatting partition " + String(this->partitionNumber) + " as LittleFS: " + String(errno));
             }
+            delete tmpFileSystem;
             return err  == 0;
     }
 
diff --git a/src/InternalStorage.h b/src/InternalStorage.h
@@ -10,66 +10,68 @@
  */
 class InternalStorage : public Arduino_UnifiedStorage {
 public:
-    
+
 
     /**
      * Constructs an InternalStorage object with default settings.
      * If no partitions are available, it restores the default partitioning scheme (See restoreDefaultPartitions() for more info).
      * If partitions are available, it sets the partition number, file system type, and partition name based on the last partition available.
-     * When using the default partitioning scheme the last partition would be the user partition. 
+     * When using the default partitioning scheme the last partition would be the user partition.
      */
     InternalStorage();
 
     /**
      * Constructs an InternalStorage object with the specified partition, name, and file system.
-     * 
+     *
      * @param partition The partition number.
      * @param name The name of the partition.
      * @param fs The desired file system (FS_FAT or FS_LITTLEFS).
      */
     InternalStorage(int partition, const char *name, FileSystems fs);
 
+    ~InternalStorage();
+
     /**
      * Initializes the internal storage.
-     * 
+     *
      * @return true if successful, false if failed.
      */
     bool begin() override;
 
     /**
      * Initializes the internal storage with the specified file system.
-     * 
+     *
      * @param fs The desired file system (FS_FAT or FS_LITTLEFS).
      * @return true if successful, false if failed.
      */
     bool begin(FileSystems fs) override;
 
     /**
      * Unmounts the internal storage.
-     * 
+     *
      * @return true if successful, false if failed.
      */
     bool unmount() override;
 
     /**
      * Retrieves the root folder of the internal storage.
-     * 
+     *
      * @return The root folder as a Folder object.
      */
     Folder getRootFolder() override;
 
 
     /**
      * Formats the internal storage with the selected file system.
-     * 
+     *
      * @return true if successful, false if failed.
      */
     bool format(FileSystems fs) override;
 
 
     /**
      * Retrieves the block device associated with the internal storage.
-     * 
+     *
      * @return The block device as a BlockDevice object.
      */
     BlockDeviceType *getBlockDevice();
@@ -86,7 +88,7 @@ class InternalStorage : public Arduino_UnifiedStorage {
      * Creates one partition spanning over the whole size of the internal storage drive erasing the existing partitions.
      * @return true if successful, false if failed.
      */
-    static bool partition(); 
+    static bool partition();
 
     /**
      * Restores the default partitioning scheme (1MB FAT32 for Certificates, 5MB FAT32 for OTA, 8MB user storage) to the internal storage drive erasing the existing partitions.
@@ -96,7 +98,7 @@ class InternalStorage : public Arduino_UnifiedStorage {
 
     /**
      * Reads the partitioning scheme from the MBR sector of the internal storage drive and returns a vector of structs of type Partition that represents the partitioning scheme
-     * @return vector of structs of type Partition 
+     * @return vector of structs of type Partition
     */
     static std::vector<Partition> readPartitions();
 
@@ -105,7 +107,7 @@ class InternalStorage : public Arduino_UnifiedStorage {
         MBRBlockDeviceType * mbrBlockDevice;
         FileSystemType * fileSystem;
         int partitionNumber;
-        char * partitionName;        
+        const char * partitionName;
         FileSystems fileSystemType;
 
 };
diff --git a/src/Partitioning.cpp b/src/Partitioning.cpp
diff --git a/src/Utils.h b/src/Utils.h

-Original file line number
+Diff line change
+}
 bool Partitioning::isPartitionSchemeValid(BlockDeviceType * blockDevice, std::vector<Partition> partitions){
 -    size_t driveSize = blockDevice -> size() / 1024; //
 +    size_t driveSize = blockDevice -> size() / 1024; //
     size_t totalSize = 0;
     for (size_t i = 1; i < partitions.size() + 1; ++i) {
+}
 bool Partitioning::createAndFormatPartitions(BlockDeviceType * blockDevice, std::vector<Partition> partitions){
+-
++
     bool success = true; // initialize to true
     int lastPartitionEnd = 0;
 std::vector<Partition> Partitioning::readPartitions(BlockDeviceType * blockDevice){
     std::vector<Partition> partitions;
+-
++
     auto returnCode = blockDevice->init();
     if (returnCode) {
         Arduino_UnifiedStorage::debugPrint("[Partitioning][readPartitions][ERROR] Unable to read the Block Device.");
     returnCode = blockDevice->read(buffer, 512 - buffer_size, buffer_size);
     if (returnCode) {
         Arduino_UnifiedStorage::debugPrint("[Partitioning][readPartitions][ERROR] Unable to read the Master Boot Record");
+-
 +        blockDevice->deinit();
         delete[] buffer;
         return partitions;
+    }
     auto table_start_offset = buffer_size - sizeof(mbrTable);
     auto table = reinterpret_cast<mbrTable*>(&buffer[table_start_offset]);
+-
++
     if (table->signature[0] != mbrMagicNumbers[0] || table->signature[1] != mbrMagicNumbers[1]) {
+-
++
         Arduino_UnifiedStorage::debugPrint("[Partitioning][readPartitions][INFO] MBR Not Found - Flash Memory doesn't have partitions.");
 +        blockDevice->deinit();
         delete[] buffer;
         return partitions;
+    }
         Partition partition;
         /*This code calculates the size of a partition in kilobytes.
 -        It takes the Logical Block Address (LBA) size of the partition,
 +        It takes the Logical Block Address (LBA) size of the partition,
         multiplies it by 4096 (the size of a block in bytes),
 -        and then shifts the result 10 bits to the right to convert it to kilobytes.
 +        and then shifts the result 10 bits to the right to convert it to kilobytes.
         */
        partition.size = (entry.lbaSize * 4096) >> 10;
         MBRBlockDeviceType * mbrBlocKDevice = new MBRBlockDeviceType(blockDevice, partitionIndex);
         FATFileSystemType  * fatProbeFileSystem =  new FATFileSystemType("probing");
         LittleFileSystemType * littleFsProbeFilesystem =  new LittleFileSystemType("probing");
+-
 -        if(fatProbeFileSystem -> mount(mbrBlocKDevice) == 0){
 -            Arduino_UnifiedStorage::debugPrint("[Partitioning][readPartitions][INFO] Partition " + String(partitionIndex) + " is formatted with FAT file system");
 -            fatProbeFileSystem -> unmount();
 -            partition.fileSystemType = FS_FAT;
 -            partitions.push_back(partition);
+-
 -        } else if (littleFsProbeFilesystem -> mount(mbrBlocKDevice) == 0){
 -            Arduino_UnifiedStorage::debugPrint("[Partitioning][readPartitions][INFO] Partition " + String(partitionIndex) + " is formatted with LittleFS file system");
 -            littleFsProbeFilesystem -> unmount();
 -            partition.fileSystemType = FS_LITTLEFS;
 -            partitions.push_back(partition);
 -        } else {
 -            Arduino_UnifiedStorage::debugPrint("[Partitioning][readPartitions][INFO] Partition " + String(partitionIndex) + " is not formatted with a recognized file system");
++
 +        if(mbrBlocKDevice && fatProbeFileSystem && littleFsProbeFilesystem)
 +        {
 +          if(fatProbeFileSystem -> mount(mbrBlocKDevice) == 0){
 +              Arduino_UnifiedStorage::debugPrint("[Partitioning][readPartitions][INFO] Partition " + String(partitionIndex) + " is formatted with FAT file system");
 +              fatProbeFileSystem -> unmount();
 +              partition.fileSystemType = FS_FAT;
 +              partitions.push_back(partition);
++
 +          } else if (littleFsProbeFilesystem -> mount(mbrBlocKDevice) == 0){
 +              Arduino_UnifiedStorage::debugPrint("[Partitioning][readPartitions][INFO] Partition " + String(partitionIndex) + " is formatted with LittleFS file system");
 +              littleFsProbeFilesystem -> unmount();
 +              partition.fileSystemType = FS_LITTLEFS;
 +              partitions.push_back(partition);
 +          } else {
 +              Arduino_UnifiedStorage::debugPrint("[Partitioning][readPartitions][INFO] Partition " + String(partitionIndex) + " is not formatted with a recognized file system");
 +          }
+        }
+-
 -    }
 +        delete mbrBlocKDevice;
 +        delete fatProbeFileSystem;
 +        delete littleFsProbeFilesystem;
 +    }
 +    blockDevice->deinit();
     delete[] buffer;
     return partitions;
+}
-Original file line number
+Diff line change
 [[gnu::unused]] static String prettyPrintFileSystemType(FileSystems f){
     if(f == 0) return "FAT";
 -    else if(f == 1) return "LitlleFS";
 +    else if(f == 1) return "LittleFS";
     else return "";
+}
     // Dynamically allocate memory for the string and copy the generated value
     char* dynamicName = new char[strlen(partitionName) + 1];
 -    strcpy(dynamicName, partitionName);
 +    if(dynamicName)
 +    {
 +      strcpy(dynamicName, partitionName);
 +    }
     return dynamicName;
+}
 +[[gnu::unused]] static void freePartitionName(const char* partitionName) {
 +    delete[] partitionName;
 +}
 [[gnu::unused]] static bool copyFolder(const char* source, const char* destination) {
   DIR* dir = opendir(source);
     size_t destinationPathLength = strlen(destination) + strlen(entry->d_name) + 1;
     char* sourcePath = new char[sourcePathLength];
 +    if(!sourcePath)
 +    {
 +      return false;
 +    }
     char* destinationPath = new char[destinationPathLength];
 +    if(!destinationPath)
 +    {
 +      delete[] sourcePath;
 +      return false;
 +    }
     snprintf(sourcePath, sourcePathLength, "%s/%s", source, entry->d_name);
     struct stat fileInfo;
     if (stat(sourcePath, &fileInfo) != 0) {
       closedir(dir);
 -      delete(sourcePath);
 -      delete(destinationPath);
 +      delete[] sourcePath;
 +      delete[] destinationPath;
       return false;
+    }
     if (S_ISDIR(fileInfo.st_mode)) {
       // Recursively copy subdirectories
       if (!copyFolder(sourcePath, destinationPath)) {
         closedir(dir);
 -        delete(sourcePath);
 -        delete(destinationPath);
 +        delete[] sourcePath;
 +        delete[] destinationPath;
         return false;
+      }
     } else {
       // Copy regular files
       FILE* sourceFile = fopen(sourcePath, "r");
       if (sourceFile == nullptr) {
         closedir(dir);
 -        delete(sourcePath);
 -        delete(destinationPath);
 +        delete[] sourcePath;
 +        delete[] destinationPath;
         return false;
+      }
       FILE* destinationFile = fopen(destinationPath, "w");
       if (destinationFile == nullptr) {
         fclose(sourceFile);
         closedir(dir);
 -        delete(sourcePath);
 -        delete(destinationPath);
 +        delete[] sourcePath;
 +        delete[] destinationPath;
         return false;
+      }
       fclose(sourceFile);
       fclose(destinationFile);
+    }
++
 +    delete[] sourcePath;
 +    delete[] destinationPath;
++
+  }
   closedir(dir);
 -#endif
 +#endif