Skip to content

Commit 8def8ae

Browse files
henrygabhenrygab
committed
Safer code
Add additional compile-time assertions Add additional comments to define limitations future maintainers should be aware of.
1 parent c51a227 commit 8def8ae

File tree

2 files changed

+164
-8
lines changed

2 files changed

+164
-8
lines changed

‎src/usb/uf2/ghostfat.c

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
#include "bootloader_settings.h"
1010
#include "bootloader.h"
1111

12+
1213
typedef struct {
1314
uint8_t JumpInstruction[3];
1415
uint8_t OEMInfo[8];
@@ -59,6 +60,7 @@ struct TextFile {
5960

6061
#define STR0(x) #x
6162
#define STR(x) STR0(x)
63+
6264
const char infoUf2File[] = //
6365
"UF2 Bootloader " UF2_VERSION "\r\n"
6466
"Model: " PRODUCT_NAME "\r\n"
@@ -81,11 +83,20 @@ static struct TextFile const info[] = {
8183
{.name = "INDEX HTM", .content = indexFile},
8284
{.name = "CURRENT UF2"},
8385
};
84-
#define NUM_INFO (sizeof(info) / sizeof(info[0]))
86+
87+
// WARNING -- code presumes each non-UF2 file content fits in single sector
88+
// Cannot programmatically statically assert .content length
89+
// for each element above.
90+
STATIC_ASSERT(ARRAY_SIZE2(indexFile) < 512);
91+
92+
93+
#define NUM_FILES (ARRAY_SIZE2(info))
94+
#define NUM_DIRENTRIES (NUM_FILES + 1) // Code adds volume label as first root directory entry
95+
8596

8697
#define UF2_SIZE (current_flash_size() * 2)
8798
#define UF2_SECTORS (UF2_SIZE / 512)
88-
#define UF2_FIRST_SECTOR (NUM_INFO + 1)
99+
#define UF2_FIRST_SECTOR (NUM_FILES + 1) // WARNING -- code presumes each non-UF2 file content fits in single sector
89100
#define UF2_LAST_SECTOR (UF2_FIRST_SECTOR + UF2_SECTORS - 1)
90101

91102
#define RESERVED_SECTORS 1
@@ -99,7 +110,8 @@ static struct TextFile const info[] = {
99110

100111
// all directory entries must fit in a single sector
101112
// because otherwise current code overflows buffer
102-
STATIC_ASSERT(NUM_INFO < (512 / sizeof(DirEntry)));
113+
STATIC_ASSERT(NUM_DIRENTRIES < (512 / sizeof(DirEntry)));
114+
// STATIC_ASSERT(NUM_DIRENTRIES < (512 / sizeof(DirEntry)) * ROOT_DIR_SECTORS);
103115

104116

105117
static FAT_BootBlock const BootBlock = {
@@ -193,8 +205,8 @@ void read_block(uint32_t block_no, uint8_t *data) {
193205
sectionIdx -= SECTORS_PER_FAT;
194206
if (sectionIdx == 0) {
195207
data[0] = 0xf0;
196-
for (int i = 1; i < NUM_INFO * 2 + 4; ++i) {
197-
data[i] = 0xff;
208+
for (int i = 1; i < NUM_FILES * 2 + 4; ++i) {
209+
data[i] = 0xff; // WARNING -- code presumes each non-UF2 file content fits in single sector
198210
}
199211
}
200212
for (int i = 0; i < 256; ++i) {
@@ -209,7 +221,7 @@ void read_block(uint32_t block_no, uint8_t *data) {
209221
DirEntry *d = (void *)data;
210222
padded_memcpy(d->name, (char const *) BootBlock.VolumeLabel, 11);
211223
d->attrs = 0x28;
212-
for (int i = 0; i < NUM_INFO; ++i) {
224+
for (int i = 0; i < NUM_FILES; ++i) {
213225
d++;
214226
struct TextFile const *inf = &info[i];
215227
d->size = inf->content ? strlen(inf->content) : UF2_SIZE;
@@ -219,10 +231,10 @@ void read_block(uint32_t block_no, uint8_t *data) {
219231
}
220232
} else {
221233
sectionIdx -= START_CLUSTERS;
222-
if (sectionIdx < NUM_INFO - 1) {
234+
if (sectionIdx < NUM_FILES - 1) {
223235
memcpy(data, info[sectionIdx].content, strlen(info[sectionIdx].content));
224236
} else {
225-
sectionIdx -= NUM_INFO - 1;
237+
sectionIdx -= NUM_FILES - 1;
226238
uint32_t addr = USER_FLASH_START + sectionIdx * 256;
227239
if (addr < USER_FLASH_START+FLASH_SIZE) {
228240
UF2_Block *bl = (void *)data;

‎src/usb/uf2/uf2.h

Lines changed: 144 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -152,3 +152,147 @@ static inline void check_uf2_handover(uint8_t *buffer, uint32_t blocks_remaining
152152
#endif
153153

154154
#endif
155+
156+
#ifndef ARRAYSIZE2_H
157+
#define ARRAYSIZE2_H
158+
159+
#ifndef __has_feature
160+
#define __has_feature(x) 0 // Compatibility with non-clang compilers.
161+
#endif
162+
163+
#if __cplusplus >= 199711L
164+
#pragma message "using Ivan J. Johnson's ARRAY_SIZE2"
165+
166+
// Works on older compilers, even Visual C++ 6....
167+
// Created by Ivan J. Johnson, March 06, 2007
168+
// See http://drdobbs.com/cpp/197800525?pgno=1
169+
//
170+
// Pseudocode:
171+
// if x is not an array
172+
// issue a compile-time error
173+
// else
174+
// use the traditional (non-typesafe) C99 COUNTOF expression
175+
//
176+
// If the argument is any of:
177+
// object of class type, such as an std::vector
178+
// floating-point type
179+
// function pointer
180+
// pointer-to-member
181+
// then the first reinterpret_cast<> is not legal (compiler error)
182+
//
183+
// The type for check1 is chosen and named to help understand
184+
// the cause of the error, because the class name is likely to
185+
// appear in the compiler error message.
186+
//
187+
// If check1 succeeds, then the argument must be one of:
188+
// an integral type
189+
// an enumerated type
190+
// a pointer to an object
191+
// an array
192+
//
193+
// Check2 expands approximately to sizeof(check_type(x, &x)),
194+
// where check_type is an overloaded function.
195+
// Because this is purely a compile-time computation,
196+
// the function is never really called or even implemented,
197+
// but it lets the compiler apply overload resolution,
198+
// which allows further type discrimination.
199+
// There are three possibilities to consider:
200+
// x is an integral type or enumerated type.
201+
// In this case, neither of the two function overloads
202+
// is a match, resulting in a compiler error.
203+
// x is a pointer to an object.
204+
// In this case, the first argument to check_type()
205+
// is a pointer and the second one is a pointer-to-pointer.
206+
// The best function match is the first overload of check_type,
207+
// the one that returns an incomplete type (Is_pointer).
208+
// However, because Is_pointer is an incomplete type,
209+
// sizeof(Is_pointer) is not a valid expression,
210+
// resulting in a compiler error.
211+
// x is an array.
212+
// In this case, the first argument to check_type()
213+
// is an array and the second is a pointer-to-array.
214+
// A pointer-to-array is *NOT* convertible to a
215+
// pointer-to-pointer, so the first overload of
216+
// check_type() is not a match.
217+
// However, an array IS convertible to a pointer,
218+
// and a pointer-to-array already is a pointer.
219+
// Any pointer is convertible to a void*,
220+
// so the second function overload is a match.
221+
// That overload returns a complete type (Is_array).
222+
// Because it's a complete type,
223+
// sizeof(Is_array) is a valid expression.
224+
// Thus, the compiler has EXCLUDED every possible type
225+
// except arrays via compilation errors before reaching
226+
// the third line.
227+
// Moreover, check1 and check2 are reduced to the value zero,
228+
// while the third line is the old type-unsafe C-style macro,
229+
// now made entirely type-safe.
230+
//
231+
// Additional benefits:
232+
// The result is itself constexpr
233+
//
234+
//
235+
#define ARRAY_SIZE2(arr) ( \
236+
0 * sizeof(reinterpret_cast<const ::Bad_arg_to_COUNTOF*>(arr)) + /*check1*/ \
237+
0 * sizeof(::Bad_arg_to_COUNTOF::check_type((arr), &(arr))) + /*check2*/ \
238+
sizeof(arr) / sizeof((arr)[0]) /* eval */ \
239+
)
240+
241+
struct Bad_arg_to_COUNTOF {
242+
class Is_pointer; // incomplete
243+
class Is_array {};
244+
template <typename T>
245+
static Is_pointer check_type(const T*, const T* const*);
246+
static Is_array check_type(const void*, const void*);
247+
};
248+
249+
#elif __cplusplus >= 201103L || /* any compiler claiming C++11 support */ \
250+
_MSC_VER >= 1900 || /* Visual C++ 2015 or higher */ \
251+
__has_feature(cxx_constexpr) /* CLang versions supporting constexp */
252+
253+
#pragma message "C++11 version ARRAY_SIZE2"
254+
255+
namespace detail
256+
{
257+
template <typename T, std::size_t N>
258+
constexpr std::size_t countof(T const (&)[N]) noexcept
259+
{
260+
return N;
261+
}
262+
} // namespace detail
263+
#define ARRAY_SIZE2(arr) detail::countof(arr)
264+
265+
#elif _MSC_VER // Visual C++ fallback
266+
267+
#pragma message "using Microsoft Visual C++ intrinsic ARRAY_SIZE2"
268+
#define ARRAY_SIZE2(arr) _countof(arr)
269+
270+
#elif __cplusplus >= 199711L && ( /* C++ 98 trick */ \
271+
defined(__INTEL_COMPILER) || \
272+
defined(__clang__) || \
273+
(defined(__GNUC__) && ( \
274+
(__GNUC__ > 4) || \
275+
(__GNUC__ == 4 && __GNUC_MINOR__ >= 4) \
276+
)))
277+
278+
#pragma message "C++98 version ARRAY_SIZE2"
279+
280+
template <typename T, std::size_t N>
281+
char(&_ArraySizeHelperRequiresArray(T(&)[N]))[N];
282+
#define ARRAY_SIZE2(x) sizeof(_ArraySizeHelperRequiresArray(x))
283+
284+
#else
285+
286+
#pragma message "Using type-unsafe version of ARRAY_SIZE2"
287+
// This is the worst-case scenario macro.
288+
// While it is valid C, it is NOT typesafe.
289+
// For example, if the parameter arr is a pointer instead of array,
290+
// the compiler will SILENTLY give a (likely) incorrect result.
291+
#define ARRAY_SIZE2(arr) sizeof(arr) / sizeof(arr[0])
292+
293+
#endif
294+
295+
296+
#endif // ARRAYSIZE2_H
297+
298+

0 commit comments

Comments
 (0)