Skip to content

Commit 09697eb

Browse files
committed
Use defines rather than magic numbers
1 parent cc829aa commit 09697eb

File tree

1 file changed

+23
-23
lines changed

1 file changed

+23
-23
lines changed

src/usb/uf2/ghostfat.c

Lines changed: 23 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,7 @@ STATIC_ASSERT(sizeof(DirEntry) == 32); // FAT
8686
STATIC_ASSERT(BPB_SECTOR_SIZE % sizeof(DirEntry) == 0); // FAT requirement
8787
STATIC_ASSERT(BPB_ROOT_DIR_ENTRIES % DIRENTRIES_PER_SECTOR == 0); // FAT requirement
8888
STATIC_ASSERT(BPB_SECTOR_SIZE * BPB_SECTORS_PER_CLUSTER <= (32*1024)); // FAT requirement (64k+ has known compatibility problems)
89+
STATIC_ASSERT(FAT_ENTRIES_PER_SECTOR == 256); // FAT requirement
8990

9091
#define STR0(x) #x
9192
#define STR(x) STR0(x)
@@ -112,32 +113,28 @@ static struct TextFile const info[] = {
112113
// current.uf2 must be the last element and its content must be NULL
113114
{.name = "CURRENT UF2", .content = NULL},
114115
};
115-
// FAT requires a final unused entry
116-
STATIC_ASSERT(ARRAY_SIZE(info) < BPB_ROOT_DIR_ENTRIES);
117-
118-
// GhostFAT presumes each non-UF2 file content fits in single sector
119-
STATIC_ASSERT(ARRAY_SIZE(infoUf2File) < BPB_SECTOR_SIZE);
120-
STATIC_ASSERT(ARRAY_SIZE(indexFile) < BPB_SECTOR_SIZE);
116+
STATIC_ASSERT(ARRAY_SIZE(infoUf2File) < BPB_SECTOR_SIZE); // GhostFAT requires files to fit in one sector
117+
STATIC_ASSERT(ARRAY_SIZE(indexFile) < BPB_SECTOR_SIZE); // GhostFAT requires files to fit in one sector
121118

122119
#define NUM_FILES (ARRAY_SIZE(info))
123120
#define NUM_DIRENTRIES (NUM_FILES + 1) // Code adds volume label as first root directory entry
124121
#define REQUIRED_ROOT_DIRECTORY_SECTORS ( ((NUM_DIRENTRIES+1) / DIRENTRIES_PER_SECTOR) + \
125122
(((NUM_DIRENTRIES+1) % DIRENTRIES_PER_SECTOR) ? 1 : 0))
126-
STATIC_ASSERT(ROOT_DIR_SECTOR_COUNT >= REQUIRED_ROOT_DIRECTORY_SECTORS); // Ensures BPB reserves sufficient entries for files
127-
STATIC_ASSERT(NUM_DIRENTRIES < (DIRENTRIES_PER_SECTOR * ROOT_DIR_SECTOR_COUNT)); // Need at least one terminating (unused) entry
128-
STATIC_ASSERT(NUM_DIRENTRIES < BPB_ROOT_DIR_ENTRIES);
129-
// all directory entries must fit in a single sector
130-
// because otherwise current code overflows buffer
131-
STATIC_ASSERT(NUM_DIRENTRIES < DIRENTRIES_PER_SECTOR);
123+
STATIC_ASSERT(ROOT_DIR_SECTOR_COUNT >= REQUIRED_ROOT_DIRECTORY_SECTORS); // FAT requirement -- Ensures BPB reserves sufficient entries for all files
124+
STATIC_ASSERT(NUM_DIRENTRIES < (DIRENTRIES_PER_SECTOR * ROOT_DIR_SECTOR_COUNT)); // FAT requirement -- end directory with unused entry
125+
STATIC_ASSERT(NUM_DIRENTRIES < BPB_ROOT_DIR_ENTRIES); // FAT requirement -- Ensures BPB reserves sufficient entries for all files
126+
STATIC_ASSERT(NUM_DIRENTRIES < DIRENTRIES_PER_SECTOR); // GhostFAT bug workaround -- else, code overflows buffer
132127

133128
#define UF2_FIRMWARE_BYTES_PER_SECTOR 256
134129
#define TRUE_USER_FLASH_SIZE (USER_FLASH_END-USER_FLASH_START)
135-
STATIC_ASSERT(TRUE_USER_FLASH_SIZE % UF2_FIRMWARE_BYTES_PER_SECTOR == 0);
130+
STATIC_ASSERT(TRUE_USER_FLASH_SIZE % UF2_FIRMWARE_BYTES_PER_SECTOR == 0); // UF2 requirement -- overall size must be integral multiple of per-sector payload?
136131

137132
#define UF2_SECTORS ( (TRUE_USER_FLASH_SIZE / UF2_FIRMWARE_BYTES_PER_SECTOR) + \
138133
((TRUE_USER_FLASH_SIZE % UF2_FIRMWARE_BYTES_PER_SECTOR) ? 1 : 0))
139134
#define UF2_SIZE (UF2_SECTORS * BPB_SECTOR_SIZE)
140135

136+
STATIC_ASSERT(UF2_SECTORS == ((UF2_SIZE/2) / 256)); // Not a requirement ... ensuring replacement of literal value is not a change
137+
141138
#define UF2_FIRST_SECTOR ((NUM_FILES + 1) * BPB_SECTORS_PER_CLUSTER) // WARNING -- code presumes each non-UF2 file content fits in single sector
142139
#define UF2_LAST_SECTOR ((UF2_FIRST_SECTOR + UF2_SECTORS - 1) * BPB_SECTORS_PER_CLUSTER)
143140

@@ -181,7 +178,7 @@ static inline bool is_uf2_block (UF2_Block const *bl)
181178
(bl->magicEnd == UF2_MAGIC_END) &&
182179
(bl->flags & UF2_FLAG_FAMILYID) &&
183180
!(bl->flags & UF2_FLAG_NOFLASH) &&
184-
(bl->payloadSize == 256) &&
181+
(bl->payloadSize == UF2_FIRMWARE_BYTES_PER_SECTOR) &&
185182
!(bl->targetAddr & 0xff);
186183
}
187184

@@ -266,12 +263,13 @@ void read_block(uint32_t block_no, uint8_t *data) {
266263
// WARNING -- code presumes only one NULL .content for .UF2 file
267264
// and all non-NULL .content fit in one sector
268265
// and requires it be the last element of the array
269-
for (uint32_t i = 1; i < NUM_FILES * 2 + 4; ++i) {
266+
uint32_t const end = (NUM_FILES * FAT_ENTRY_SIZE) + (2 * FAT_ENTRY_SIZE);
267+
for (uint32_t i = 1; i < end; ++i) {
270268
data[i] = 0xff;
271269
}
272270
}
273-
for (uint32_t i = 0; i < 256; ++i) { // Generate the FAT chain for the firmware "file"
274-
uint32_t v = sectionIdx * 256 + i;
271+
for (uint32_t i = 0; i < FAT_ENTRIES_PER_SECTOR; ++i) { // Generate the FAT chain for the firmware "file"
272+
uint32_t v = (sectionIdx * FAT_ENTRIES_PER_SECTOR) + i;
275273
if (UF2_FIRST_SECTOR <= v && v <= UF2_LAST_SECTOR)
276274
((uint16_t *)(void *)data)[i] = v == UF2_LAST_SECTOR ? 0xffff : v + 1;
277275
}
@@ -302,35 +300,37 @@ void read_block(uint32_t block_no, uint8_t *data) {
302300
d->createTime = __DOSTIME__;
303301
d->createDate = __DOSDATE__;
304302
d->lastAccessDate = __DOSDATE__;
305-
d->highStartCluster = startCluster >> 8;
303+
d->highStartCluster = startCluster >> 8; // BUGBUG -- shouldn't this be 16?
306304
// DIR_WrtTime and DIR_WrtDate must be supported
307305
d->updateTime = __DOSTIME__;
308306
d->updateDate = __DOSDATE__;
309-
d->startCluster = startCluster & 0xFF;
307+
d->startCluster = startCluster & 0xFF; // BUGBUG -- shouldn't this be 0xFFFF?
310308
d->size = (inf->content ? strlen(inf->content) : UF2_SIZE);
311309
}
312310

313-
} else {
311+
} else if (block_no < BPB_TOTAL_SECTORS) {
312+
314313
sectionIdx -= FS_START_CLUSTERS_SECTOR;
315314
if (sectionIdx < NUM_FILES - 1) {
316315
memcpy(data, info[sectionIdx].content, strlen(info[sectionIdx].content));
317316
} else { // generate the UF2 file data on-the-fly
318317
sectionIdx -= NUM_FILES - 1;
319-
uint32_t addr = USER_FLASH_START + sectionIdx * 256;
318+
uint32_t addr = USER_FLASH_START + (sectionIdx * UF2_FIRMWARE_BYTES_PER_SECTOR);
320319
if (addr < CFG_UF2_FLASH_SIZE) {
321320
UF2_Block *bl = (void *)data;
322321
bl->magicStart0 = UF2_MAGIC_START0;
323322
bl->magicStart1 = UF2_MAGIC_START1;
324323
bl->magicEnd = UF2_MAGIC_END;
325324
bl->blockNo = sectionIdx;
326-
bl->numBlocks = (UF2_SIZE/2) / 256;
325+
bl->numBlocks = UF2_SECTORS;
327326
bl->targetAddr = addr;
328-
bl->payloadSize = 256;
327+
bl->payloadSize = UF2_FIRMWARE_BYTES_PER_SECTOR;
329328
bl->flags = UF2_FLAG_FAMILYID;
330329
bl->familyID = CFG_UF2_FAMILY_APP_ID;
331330
memcpy(bl->data, (void *)addr, bl->payloadSize);
332331
}
333332
}
333+
334334
}
335335
}
336336

0 commit comments

Comments
 (0)