Safer code
Add additional compile-time assertions Add additional comments to define limitations future maintainers should be aware of.
This commit is contained in:
		| @@ -9,6 +9,7 @@ | |||||||
| #include "bootloader_settings.h" | #include "bootloader_settings.h" | ||||||
| #include "bootloader.h" | #include "bootloader.h" | ||||||
|  |  | ||||||
|  |  | ||||||
| typedef struct { | typedef struct { | ||||||
|     uint8_t JumpInstruction[3]; |     uint8_t JumpInstruction[3]; | ||||||
|     uint8_t OEMInfo[8]; |     uint8_t OEMInfo[8]; | ||||||
| @@ -59,6 +60,7 @@ struct TextFile { | |||||||
|  |  | ||||||
| #define STR0(x) #x | #define STR0(x) #x | ||||||
| #define STR(x) STR0(x) | #define STR(x) STR0(x) | ||||||
|  |  | ||||||
| const char infoUf2File[] = // | const char infoUf2File[] = // | ||||||
|     "UF2 Bootloader " UF2_VERSION "\r\n" |     "UF2 Bootloader " UF2_VERSION "\r\n" | ||||||
|     "Model: " PRODUCT_NAME "\r\n" |     "Model: " PRODUCT_NAME "\r\n" | ||||||
| @@ -81,11 +83,20 @@ static struct TextFile const info[] = { | |||||||
|     {.name = "INDEX   HTM", .content = indexFile}, |     {.name = "INDEX   HTM", .content = indexFile}, | ||||||
|     {.name = "CURRENT UF2"}, |     {.name = "CURRENT UF2"}, | ||||||
| }; | }; | ||||||
| #define NUM_INFO (sizeof(info) / sizeof(info[0])) |  | ||||||
|  | // WARNING -- code presumes each non-UF2 file content fits in single sector | ||||||
|  | //            Cannot programmatically statically assert .content length | ||||||
|  | //            for each element above. | ||||||
|  | STATIC_ASSERT(ARRAY_SIZE2(indexFile) < 512); | ||||||
|  |  | ||||||
|  |  | ||||||
|  | #define NUM_FILES (ARRAY_SIZE2(info)) | ||||||
|  | #define NUM_DIRENTRIES (NUM_FILES + 1) // Code adds volume label as first root directory entry | ||||||
|  |  | ||||||
|  |  | ||||||
| #define UF2_SIZE           (current_flash_size() * 2) | #define UF2_SIZE           (current_flash_size() * 2) | ||||||
| #define UF2_SECTORS        (UF2_SIZE / 512) | #define UF2_SECTORS        (UF2_SIZE / 512) | ||||||
| #define UF2_FIRST_SECTOR   (NUM_INFO + 1) | #define UF2_FIRST_SECTOR   (NUM_FILES + 1) // WARNING -- code presumes each non-UF2 file content fits in single sector | ||||||
| #define UF2_LAST_SECTOR    (UF2_FIRST_SECTOR + UF2_SECTORS - 1) | #define UF2_LAST_SECTOR    (UF2_FIRST_SECTOR + UF2_SECTORS - 1) | ||||||
|  |  | ||||||
| #define RESERVED_SECTORS   1 | #define RESERVED_SECTORS   1 | ||||||
| @@ -99,7 +110,8 @@ static struct TextFile const info[] = { | |||||||
|  |  | ||||||
| // all directory entries must fit in a single sector | // all directory entries must fit in a single sector | ||||||
| // because otherwise current code overflows buffer | // because otherwise current code overflows buffer | ||||||
| STATIC_ASSERT(NUM_INFO < (512 / sizeof(DirEntry))); | STATIC_ASSERT(NUM_DIRENTRIES < (512 / sizeof(DirEntry))); | ||||||
|  | // STATIC_ASSERT(NUM_DIRENTRIES < (512 / sizeof(DirEntry)) * ROOT_DIR_SECTORS); | ||||||
|  |  | ||||||
|  |  | ||||||
| static FAT_BootBlock const BootBlock = { | static FAT_BootBlock const BootBlock = { | ||||||
| @@ -193,8 +205,8 @@ void read_block(uint32_t block_no, uint8_t *data) { | |||||||
|             sectionIdx -= SECTORS_PER_FAT; |             sectionIdx -= SECTORS_PER_FAT; | ||||||
|         if (sectionIdx == 0) { |         if (sectionIdx == 0) { | ||||||
|             data[0] = 0xf0; |             data[0] = 0xf0; | ||||||
|             for (int i = 1; i < NUM_INFO * 2 + 4; ++i) { |             for (int i = 1; i < NUM_FILES * 2 + 4; ++i) { | ||||||
|                 data[i] = 0xff; |                 data[i] = 0xff; // WARNING -- code presumes each non-UF2 file content fits in single sector | ||||||
|             } |             } | ||||||
|         } |         } | ||||||
|         for (int i = 0; i < 256; ++i) { |         for (int i = 0; i < 256; ++i) { | ||||||
| @@ -209,7 +221,7 @@ void read_block(uint32_t block_no, uint8_t *data) { | |||||||
|             DirEntry *d = (void *)data; |             DirEntry *d = (void *)data; | ||||||
|             padded_memcpy(d->name, (char const *) BootBlock.VolumeLabel, 11); |             padded_memcpy(d->name, (char const *) BootBlock.VolumeLabel, 11); | ||||||
|             d->attrs = 0x28; |             d->attrs = 0x28; | ||||||
|             for (int i = 0; i < NUM_INFO; ++i) { |             for (int i = 0; i < NUM_FILES; ++i) { | ||||||
|                 d++; |                 d++; | ||||||
|                 struct TextFile const *inf = &info[i]; |                 struct TextFile const *inf = &info[i]; | ||||||
|                 d->size = inf->content ? strlen(inf->content) : UF2_SIZE; |                 d->size = inf->content ? strlen(inf->content) : UF2_SIZE; | ||||||
| @@ -219,10 +231,10 @@ void read_block(uint32_t block_no, uint8_t *data) { | |||||||
|         } |         } | ||||||
|     } else { |     } else { | ||||||
|         sectionIdx -= START_CLUSTERS; |         sectionIdx -= START_CLUSTERS; | ||||||
|         if (sectionIdx < NUM_INFO - 1) { |         if (sectionIdx < NUM_FILES - 1) { | ||||||
|             memcpy(data, info[sectionIdx].content, strlen(info[sectionIdx].content)); |             memcpy(data, info[sectionIdx].content, strlen(info[sectionIdx].content)); | ||||||
|         } else { |         } else { | ||||||
|             sectionIdx -= NUM_INFO - 1; |             sectionIdx -= NUM_FILES - 1; | ||||||
|             uint32_t addr = USER_FLASH_START + sectionIdx * 256; |             uint32_t addr = USER_FLASH_START + sectionIdx * 256; | ||||||
|             if (addr < USER_FLASH_START+FLASH_SIZE) { |             if (addr < USER_FLASH_START+FLASH_SIZE) { | ||||||
|                 UF2_Block *bl = (void *)data; |                 UF2_Block *bl = (void *)data; | ||||||
|   | |||||||
| @@ -152,3 +152,147 @@ static inline void check_uf2_handover(uint8_t *buffer, uint32_t blocks_remaining | |||||||
| #endif | #endif | ||||||
|  |  | ||||||
| #endif | #endif | ||||||
|  |  | ||||||
|  | #ifndef ARRAYSIZE2_H | ||||||
|  | #define ARRAYSIZE2_H | ||||||
|  |  | ||||||
|  | #ifndef __has_feature | ||||||
|  |     #define __has_feature(x) 0 // Compatibility with non-clang compilers. | ||||||
|  | #endif | ||||||
|  |  | ||||||
|  | #if __cplusplus >= 199711L | ||||||
|  |     #pragma message "using Ivan J. Johnson's ARRAY_SIZE2" | ||||||
|  |  | ||||||
|  |     // Works on older compilers, even Visual C++ 6.... | ||||||
|  |     // Created by Ivan J. Johnson, March 06, 2007 | ||||||
|  |     // See http://drdobbs.com/cpp/197800525?pgno=1 | ||||||
|  |     // | ||||||
|  |     // Pseudocode: | ||||||
|  |     // if x is not an array | ||||||
|  |     //   issue a compile-time error | ||||||
|  |     // else | ||||||
|  |     //   use the traditional (non-typesafe) C99 COUNTOF expression | ||||||
|  |     // | ||||||
|  |     // If the argument is any of: | ||||||
|  |     //    object of class type, such as an std::vector | ||||||
|  |     //    floating-point type | ||||||
|  |     //    function pointer | ||||||
|  |     //    pointer-to-member | ||||||
|  |     // then the first reinterpret_cast<> is not legal (compiler error) | ||||||
|  |     // | ||||||
|  |     // The type for check1 is chosen and named to help understand | ||||||
|  |     // the cause of the error, because the class name is likely to | ||||||
|  |     // appear in the compiler error message. | ||||||
|  |     // | ||||||
|  |     // If check1 succeeds, then the argument must be one of: | ||||||
|  |     //    an integral type | ||||||
|  |     //    an enumerated type | ||||||
|  |     //    a pointer to an object | ||||||
|  |     //    an array | ||||||
|  |     // | ||||||
|  |     // Check2 expands approximately to sizeof(check_type(x, &x)), | ||||||
|  |     // where check_type is an overloaded function. | ||||||
|  |     // Because this is purely a compile-time computation, | ||||||
|  |     // the function is never really called or even implemented, | ||||||
|  |     // but it lets the compiler apply overload resolution, | ||||||
|  |     // which allows further type discrimination. | ||||||
|  |     // There are three possibilities to consider: | ||||||
|  |     //    x is an integral type or enumerated type. | ||||||
|  |     //      In this case, neither of the two function overloads | ||||||
|  |     //      is a match, resulting in a compiler error. | ||||||
|  |     //    x is a pointer to an object. | ||||||
|  |     //      In this case, the first argument to check_type() | ||||||
|  |     //      is a pointer and the second one is a pointer-to-pointer. | ||||||
|  |     //      The best function match is the first overload of check_type, | ||||||
|  |     //      the one that returns an incomplete type (Is_pointer). | ||||||
|  |     //      However, because Is_pointer is an incomplete type, | ||||||
|  |     //      sizeof(Is_pointer) is not a valid expression, | ||||||
|  |     //      resulting in a compiler error. | ||||||
|  |     //    x is an array. | ||||||
|  |     //      In this case, the first argument to check_type() | ||||||
|  |     //      is an array and the second is a pointer-to-array. | ||||||
|  |     //      A pointer-to-array is *NOT* convertible to a | ||||||
|  |     //      pointer-to-pointer, so the first overload of | ||||||
|  |     //      check_type() is not a match. | ||||||
|  |     //      However, an array IS convertible to a pointer, | ||||||
|  |     //      and a pointer-to-array already is a pointer. | ||||||
|  |     //      Any pointer is convertible to a void*, | ||||||
|  |     //      so the second function overload is a match. | ||||||
|  |     //      That overload returns a complete type (Is_array). | ||||||
|  |     //      Because it's a complete type, | ||||||
|  |     //      sizeof(Is_array) is a valid expression. | ||||||
|  |     // Thus, the compiler has EXCLUDED every possible type | ||||||
|  |     // except arrays via compilation errors before reaching | ||||||
|  |     // the third line. | ||||||
|  |     // Moreover, check1 and check2 are reduced to the value zero, | ||||||
|  |     // while the third line is the old type-unsafe C-style macro, | ||||||
|  |     // now made entirely type-safe. | ||||||
|  |     //  | ||||||
|  |     // Additional benefits: | ||||||
|  |     // The result is itself constexpr | ||||||
|  |     //  | ||||||
|  |     // | ||||||
|  |     #define ARRAY_SIZE2(arr) ( \ | ||||||
|  |        0 * sizeof(reinterpret_cast<const ::Bad_arg_to_COUNTOF*>(arr)) + /*check1*/ \ | ||||||
|  |        0 * sizeof(::Bad_arg_to_COUNTOF::check_type((arr), &(arr)))    + /*check2*/ \ | ||||||
|  |        sizeof(arr) / sizeof((arr)[0])                                   /* eval */ \ | ||||||
|  |        ) | ||||||
|  |  | ||||||
|  |     struct Bad_arg_to_COUNTOF { | ||||||
|  |        class Is_pointer; // incomplete | ||||||
|  |        class Is_array {}; | ||||||
|  |        template <typename T> | ||||||
|  |        static Is_pointer check_type(const T*, const T* const*); | ||||||
|  |        static Is_array check_type(const void*, const void*); | ||||||
|  |     }; | ||||||
|  |  | ||||||
|  | #elif __cplusplus >= 201103L ||  /* any compiler claiming C++11 support */ \ | ||||||
|  |     _MSC_VER >= 1900 ||          /* Visual C++ 2015 or higher           */ \ | ||||||
|  |     __has_feature(cxx_constexpr) /* CLang versions supporting constexp  */ | ||||||
|  |  | ||||||
|  |     #pragma message "C++11 version ARRAY_SIZE2" | ||||||
|  |  | ||||||
|  |     namespace detail | ||||||
|  |     { | ||||||
|  |         template <typename T, std::size_t N> | ||||||
|  |         constexpr std::size_t countof(T const (&)[N]) noexcept | ||||||
|  |         { | ||||||
|  |             return N; | ||||||
|  |         } | ||||||
|  |     } // namespace detail | ||||||
|  |     #define ARRAY_SIZE2(arr) detail::countof(arr) | ||||||
|  |  | ||||||
|  | #elif _MSC_VER // Visual C++ fallback | ||||||
|  |  | ||||||
|  |     #pragma message "using Microsoft Visual C++ intrinsic ARRAY_SIZE2" | ||||||
|  |     #define ARRAY_SIZE2(arr) _countof(arr) | ||||||
|  |  | ||||||
|  | #elif __cplusplus >= 199711L && ( /* C++ 98 trick */ \ | ||||||
|  |     defined(__INTEL_COMPILER) ||                     \ | ||||||
|  |     defined(__clang__) ||                            \ | ||||||
|  |     (defined(__GNUC__) && (                          \ | ||||||
|  |         (__GNUC__ > 4) ||                            \ | ||||||
|  |         (__GNUC__ == 4 && __GNUC_MINOR__ >= 4)       \ | ||||||
|  |     ))) | ||||||
|  |  | ||||||
|  |     #pragma message "C++98 version ARRAY_SIZE2" | ||||||
|  |  | ||||||
|  |     template <typename T, std::size_t N> | ||||||
|  |     char(&_ArraySizeHelperRequiresArray(T(&)[N]))[N]; | ||||||
|  |     #define ARRAY_SIZE2(x) sizeof(_ArraySizeHelperRequiresArray(x)) | ||||||
|  |  | ||||||
|  | #else | ||||||
|  |  | ||||||
|  |     #pragma message "Using type-unsafe version of ARRAY_SIZE2" | ||||||
|  |     // This is the worst-case scenario macro. | ||||||
|  |     // While it is valid C, it is NOT typesafe. | ||||||
|  |     // For example, if the parameter arr is a pointer instead of array, | ||||||
|  |     // the compiler will SILENTLY give a (likely) incorrect result.  | ||||||
|  |     #define ARRAY_SIZE2(arr) sizeof(arr) / sizeof(arr[0]) | ||||||
|  |  | ||||||
|  | #endif | ||||||
|  |  | ||||||
|  |  | ||||||
|  | #endif // ARRAYSIZE2_H | ||||||
|  |  | ||||||
|  |  | ||||||
|   | |||||||
		Reference in New Issue
	
	Block a user