fix: Minor changes and fixes

Will be squashed

Author: adokitkat

esp_ext_part_tables/CMakeLists.txt

@@ -3,4 +3,5 @@ set(srcs "src/esp_ext_part_tables.c"
 
 idf_component_register(SRCS ${srcs}
                     INCLUDE_DIRS "include"
+                    PRIV_INCLUDE_DIRS "private_include"
                     REQUIRES "log" "esp_common")

esp_ext_part_tables/include/esp_ext_part_tables.h

@@ -9,7 +9,6 @@
 #include <stdint.h>
 #include <stdbool.h>
 #include <stddef.h>
-#include <sys/lock.h>
 #include "esp_err.h"
 
 #if __has_include(<bsd/sys/queue.h>)
@@ -37,6 +36,7 @@ typedef enum {
 
 typedef enum {
     ESP_EXT_PART_TYPE_UNKNOWN = 0,
+    ESP_EXT_PART_TYPE_FAT_ANY, /*!< FAT partition (any type) */
     ESP_EXT_PART_TYPE_FAT12,
     ESP_EXT_PART_TYPE_FAT16, /*!< FAT16 with LBA addressing */
     ESP_EXT_PART_TYPE_FAT32, /*!< FAT32 with LBA addressing */
@@ -68,12 +68,12 @@ typedef struct {
 } esp_ext_part_list_signature_t;
 
 typedef struct {
-    uint64_t address;
-    uint64_t sector_count;
-    uint32_t extra;
+    uint64_t address; /*!< Start address in LBA */
+    uint64_t size; /*!< Size in bytes */
+    uint32_t extra; /*!< Extra information (e.g. LittleFS block size stored in CHS hack, etc.) */
     char* label;
-    esp_ext_part_flags_t flags;
-    esp_ext_part_type_known_t type;
+    esp_ext_part_flags_t flags; /*!< Flags for the partition */
+    esp_ext_part_type_known_t type; /*!< Known partition type for this component */
 } esp_ext_part_t;
 
 typedef struct esp_ext_part_list_item_ {
@@ -83,8 +83,8 @@ typedef struct esp_ext_part_list_item_ {
 
 typedef struct {
     esp_ext_part_list_signature_t signature; /*!< Disk signature or identifier */
-    SLIST_HEAD(esp_ext_part_list_head_, esp_ext_part_list_item_) head;
-    esp_ext_part_list_flags_t flags;
+    SLIST_HEAD(esp_ext_part_list_head_, esp_ext_part_list_item_) head; /*!< Head of the partition list */
+    esp_ext_part_list_flags_t flags; /*!< Flags for the partition list */
     esp_ext_part_sector_size_t sector_size; /*!< Sector size (storage medium property) */
 } esp_ext_part_list_t;
 
@@ -168,6 +168,7 @@ esp_err_t esp_ext_part_list_signature_get(esp_ext_part_list_t* part_list, void*
  *     - ESP_ERR_NOT_SUPPORTED: Unsupported signature type.
  */
 esp_err_t esp_ext_part_list_signature_set(esp_ext_part_list_t* part_list, const void* signature, esp_ext_part_signature_type_t type);
+
 #ifdef __cplusplus
 }
 #endif

esp_ext_part_tables/include/esp_mbr.h

@@ -68,12 +68,6 @@ typedef struct {
 } mbr_t;
 #pragma pack(pop)
 
-// Helper functions for MBR CHS conversion and LBA alignment
-void mbr_set_chs(uint8_t chs[3], uint32_t val);
-uint32_t mbr_get_chs(const uint8_t chs[3]);
-void lba_to_chs(uint8_t chs[3], uint32_t lba);
-uint32_t lba_align(uint32_t lba, esp_ext_part_sector_size_t sector_size, esp_ext_part_align_t alignment);
-
 typedef struct {
     esp_ext_part_sector_size_t sector_size; // Sector size hint, pulled from a storage device driver query
 } esp_mbr_parse_extra_args_t;
@@ -126,6 +120,7 @@ esp_err_t esp_mbr_parse(void* mbr_buf,
  *     - ESP_OK:              Generation was successful.
  *     - ESP_ERR_INVALID_ARG: Invalid arguments were provided.
  *     - ESP_ERR_INVALID_STATE: Error filling partition entry.
+ *     - ESP_ERR_NOT_SUPPORTED: Partition address or size (sector count) exceeds 32-bit limit of MBR.
  *     - Other error codes from `esp_ext_part_list_insert` or related routines.
  */
 esp_err_t esp_mbr_generate(mbr_t* mbr,

esp_ext_part_tables/private_include/esp_ext_part_tables_private.h

@@ -0,0 +1,22 @@
+/*
+ * SPDX-FileCopyrightText: 2025 Espressif Systems (Shanghai) CO LTD
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+#pragma once
+
+#include <stdint.h>
+#include "esp_ext_part_tables.h"
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+uint64_t esp_ext_part_bytes_to_sector_count(uint64_t total_bytes, esp_ext_part_sector_size_t sector_size);
+
+uint64_t esp_ext_part_sector_count_to_bytes(uint64_t sector_count, esp_ext_part_sector_size_t sector_size);
+
+#ifdef __cplusplus
+}
+#endif

esp_ext_part_tables/private_include/esp_mbr_private.h

@@ -0,0 +1,33 @@
+/*
+ * SPDX-FileCopyrightText: 2025 Espressif Systems (Shanghai) CO LTD
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+#pragma once
+
+#include <stdint.h>
+#include "esp_ext_part_tables.h"
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+#define MBR_CHS_HEADS 255
+#define MBR_CHS_SECTORS_PER_TRACK 63
+#define MBR_CHS_MAX_CYLINDER 1023
+#define MBR_CHS_MAX_HEAD 254
+#define MBR_CHS_MAX_SECTOR 63
+
+// Helper functions for MBR CHS conversion and LBA alignment
+void esp_mbr_chs_arr_val_set(uint8_t chs[3], uint32_t val);
+
+uint32_t esp_mbr_chs_arr_val_get(const uint8_t chs[3]);
+
+void esp_mbr_lba_to_chs_arr(uint8_t chs[3], uint32_t lba);
+
+uint32_t esp_mbr_lba_align(uint32_t lba, esp_ext_part_sector_size_t sector_size, esp_ext_part_align_t alignment);
+
+#ifdef __cplusplus
+}
+#endif

esp_ext_part_tables/src/esp_ext_part_tables.c

@@ -8,17 +8,34 @@
 #include <assert.h>
 #include <string.h>
 #include <stdio.h>
-#include <sys/lock.h>
+#include "freertos/FreeRTOS.h"
+#include "freertos/task.h"
 #include "esp_err.h"
 #include "esp_log.h"
+
 #include "esp_ext_part_tables.h"
+#include "esp_ext_part_tables_private.h"
 
 #if __has_include(<bsd/sys/queue.h>)
 #include <bsd/sys/queue.h>
 #else
 #include "sys/queue.h"
 #endif
 
+uint64_t esp_ext_part_bytes_to_sector_count(uint64_t total_bytes, esp_ext_part_sector_size_t sector_size)
+{
+    if (sector_size == ESP_EXT_PART_SECTOR_SIZE_UNKNOWN) {
+        return 0; // Avoid division by zero
+    }
+    // Ceiling division for integers: (a + b - 1) / b
+    return ((total_bytes + (uint64_t) sector_size - 1) / (uint64_t) sector_size);
+}
+
+uint64_t esp_ext_part_sector_count_to_bytes(uint64_t sector_count, esp_ext_part_sector_size_t sector_size)
+{
+    return sector_count * (uint64_t) sector_size;
+}
+
 esp_err_t esp_ext_part_list_deinit(esp_ext_part_list_t* part_list)
 {
     if (part_list == NULL) {

esp_ext_part_tables/src/esp_mbr.c

@@ -11,8 +11,11 @@
 #include "esp_err.h"
 #include "esp_log.h"
 #include "esp_random.h"
+
 #include "esp_ext_part_tables.h"
+#include "esp_ext_part_tables_private.h"
 #include "esp_mbr.h"
+#include "esp_mbr_private.h"
 
 #if __has_include(<bsd/sys/queue.h>)
 #include <bsd/sys/queue.h>
@@ -22,25 +25,19 @@
 
 static const char* TAG = "esp_mbr";
 
-#define MBR_CHS_HEADS 255
-#define MBR_CHS_SECTORS_PER_TRACK 63
-#define MBR_CHS_MAX_CYLINDER 1023
-#define MBR_CHS_MAX_HEAD 254
-#define MBR_CHS_MAX_SECTOR 63
-
-void mbr_set_chs(uint8_t chs[3], uint32_t val)
+void esp_mbr_chs_arr_val_set(uint8_t chs[3], uint32_t val)
 {
     chs[0] = val & 0xFF;
     chs[1] = (val >> 8) & 0xFF;
     chs[2] = (val >> 16) & 0xFF;
 }
 
-uint32_t mbr_get_chs(const uint8_t chs[3])
+uint32_t esp_mbr_chs_arr_val_get(const uint8_t chs[3])
 {
     return chs[0] | (chs[1] << 8) | (chs[2] << 16);
 }
 
-void lba_to_chs(uint8_t chs[3], uint32_t lba)
+void esp_mbr_lba_to_chs_arr(uint8_t chs[3], uint32_t lba)
 {
     uint16_t cylinder;
     uint8_t head;
@@ -75,7 +72,7 @@ void lba_to_chs(uint8_t chs[3], uint32_t lba)
     }
 }
 
-uint32_t lba_align(uint32_t lba, esp_ext_part_sector_size_t sector_size, esp_ext_part_align_t alignment)
+uint32_t esp_mbr_lba_align(uint32_t lba, esp_ext_part_sector_size_t sector_size, esp_ext_part_align_t alignment)
 {
     if (sector_size == 0 || alignment == 0) {
         return lba; // No alignment
@@ -93,6 +90,18 @@ static uint8_t ext_part_type_known_to_mbr_type(esp_ext_part_type_known_t type)
         return 0x06; // FAT16B with LBA addressing
     case ESP_EXT_PART_TYPE_FAT32:
         return 0x0C; // FAT32 with LBA addressing
+    /* 
+    LittleFS is not a standard MBR partition type, but we can use a custom type `0xC3`, which is not usually used nowadays.
+    This allows us to identify LittleFS partitions in the MBR.
+
+    Explanation why `0xC3` was chosen:
+    0xC    3
+      1100 0011
+      ↑↑ ↑ ↑↑↑↑ 
+      └│─│─┴┴┴┴── 0x83 => a modern filesystem (e.g. Linux)
+       └─│─────── 0x40 => CHS used as LittleFS block size
+         └─────── 0x10 => a hidden filesystem
+    */
     case ESP_EXT_PART_TYPE_LITTLEFS:
         return 0xC3; // Possibly LittleFS (MBR CHS field => LittleFS block size hack)
     case ESP_EXT_PART_TYPE_EXFAT_OR_NTFS: // Not supported, but we can return a type for it
@@ -173,7 +182,7 @@ static void ext_part_list_item_do_extra(esp_ext_part_list_item_t* item, mbr_part
     switch (item->info.type) { // Parsed type
     case ESP_EXT_PART_TYPE_LITTLEFS:
         item->info.flags |= ESP_EXT_PART_FLAG_EXTRA; // Set the extra flag to indicate that this partition has extra information
-        item->info.extra = mbr_get_chs(partition->chs_start); // Put LittleFS block size which was stored in `chs_start` to `extra` field
+        item->info.extra = esp_mbr_chs_arr_val_get(partition->chs_start); // Put LittleFS block size which was stored in `chs_start` to `extra` field
         break;
     default:
         break;
@@ -195,11 +204,13 @@ esp_err_t esp_mbr_parse(void* mbr_buf,
         return ESP_ERR_NOT_FOUND;
     }
 
+    // Set defaults
+    part_list->sector_size = ESP_EXT_PART_SECTOR_SIZE_512B; // Default sector size
+
+    // Load extra arguments if provided
     if (extra_args) {
         if (extra_args->sector_size != ESP_EXT_PART_SECTOR_SIZE_UNKNOWN) {
             part_list->sector_size = extra_args->sector_size; // Use the sector size hint from extra_args
-        } else {
-            part_list->sector_size = ESP_EXT_PART_SECTOR_SIZE_512B; // Default sector size
         }
     }
 
@@ -240,7 +251,7 @@ esp_err_t esp_mbr_parse(void* mbr_buf,
         esp_ext_part_list_item_t item = {
             .info = {
                 .address = partition->lba_start,
-                .sector_count = partition->sector_count,
+                .size = esp_ext_part_sector_count_to_bytes((uint64_t) partition->sector_count, part_list->sector_size),
                 .extra = 0,
                 .label = NULL, // MBR does not have labels
                 .flags = ESP_EXT_PART_FLAG_NONE,
@@ -275,14 +286,14 @@ static bool mbr_partition_fill(mbr_partition_t* partition, esp_ext_part_list_ite
     case ESP_EXT_PART_TYPE_FAT16:
     case ESP_EXT_PART_TYPE_FAT32:
         // Set CHS values based on LBA start and end
-        lba_to_chs(partition->chs_start, lba_start);
-        lba_to_chs(partition->chs_end, lba_end);
+        esp_mbr_lba_to_chs_arr(partition->chs_start, lba_start);
+        esp_mbr_lba_to_chs_arr(partition->chs_end, lba_end);
         break;
     case ESP_EXT_PART_TYPE_LITTLEFS:
         // Use `chs_start` to store LittleFS block size (if stored in `extra` field)
         if (item->info.extra != 0) {
             // If the extra flag is set, use the extra field to store the LittleFS block size
-            mbr_set_chs(partition->chs_start, item->info.extra);
+            esp_mbr_chs_arr_val_set(partition->chs_start, item->info.extra);
 
             if (!(item->info.flags & ESP_EXT_PART_FLAG_EXTRA)) {
                 // If the extra flag is not set but the extra field is set, log a warning
@@ -307,12 +318,14 @@ esp_err_t esp_mbr_generate(mbr_t* mbr,
         return ESP_ERR_INVALID_ARG;
     }
 
+    // Set default arguments for MBR generation
     esp_mbr_generate_extra_args_t args = {
         .sector_size = part_list->sector_size != ESP_EXT_PART_SECTOR_SIZE_UNKNOWN ? part_list->sector_size : ESP_EXT_PART_SECTOR_SIZE_512B, // Default sector size
         .alignment = ESP_EXT_PART_ALIGN_1MiB, // Default alignment
         .keep_signature = false, // Default is to generate a new disk signature
     };
 
+    // Load extra arguments if provided
     if (extra_args) {
         if (extra_args->sector_size != ESP_EXT_PART_SECTOR_SIZE_UNKNOWN) {
             args.sector_size = extra_args->sector_size;
@@ -342,15 +355,23 @@ esp_err_t esp_mbr_generate(mbr_t* mbr,
     mbr_partition_t* partition = NULL;
     esp_ext_part_list_item_t *it = NULL;
     int i = 0;
+    uint64_t sector_count;
     SLIST_FOREACH(it, &part_list->head, next) {
+        // Check if we have enough space in the MBR partition table
+        sector_count = esp_ext_part_bytes_to_sector_count(it->info.size, args.sector_size);
+        if (it->info.address > UINT32_MAX || sector_count > UINT32_MAX) {
+            ESP_LOGE(TAG, "Partition address or size exceeds 32-bit limit of MBR");
+            return ESP_ERR_NOT_SUPPORTED; // Address or size too large for MBR
+        }
+
         partition = (mbr_partition_t*) &mbr->partition_table[i];
         i += 1;
 
         if (it->info.flags & ESP_EXT_PART_FLAG_ACTIVE) {
             partition->status = MBR_PARTITION_STATUS_ACTIVE;
         }
-        partition->lba_start = lba_align(it->info.address, args.sector_size, args.alignment);
-        partition->sector_count = it->info.sector_count;
+        partition->lba_start = esp_mbr_lba_align((uint32_t) it->info.address, args.sector_size, args.alignment);
+        partition->sector_count = (uint32_t) sector_count;
         partition->type = ext_part_type_known_to_mbr_type(it->info.type);
 
         if (mbr_partition_fill(partition, it) == false) {

esp_ext_part_tables/test_apps/main/CMakeLists.txt

@@ -1,3 +1,3 @@
 idf_component_register(SRC_DIRS "."
-                    PRIV_INCLUDE_DIRS "."
+                    PRIV_INCLUDE_DIRS "." "../../private_include"
                     PRIV_REQUIRES esp_ext_part_tables unity)