Copilot commented on code in PR #3469:
URL: https://github.com/apache/nuttx-apps/pull/3469#discussion_r3177847578
##########
fsutils/mkfatfs/configfat.c:
##########
@@ -270,13 +247,8 @@ static inline uint32_t
mkfatfs_nfatsect32(FAR struct fat_format_s *fmt, FAR struct fat_var_s *var,
uint32_t navailsects)
{
-#ifdef CONFIG_HAVE_LONG_LONG
uint64_t denom;
uint64_t number;
-#else
- uint32_t denom;
- uint32_t number;
-#endif
Review Comment:
`mkfatfs_nfatsect32()` now declares `denom/number` as `uint64_t`, but
`number` is later computed via `navailsects + ...` where `navailsects` is
`uint32_t`. That addition can wrap in 32-bit before being assigned to
`uint64_t`, producing an incorrect FAT size on large media. Cast `navailsects`
(and keep the added terms in 64-bit) at the assignment sites so the computation
is truly 64-bit.
##########
fsutils/mkfatfs/configfat.c:
##########
@@ -201,13 +183,8 @@ static inline uint32_t
mkfatfs_nfatsect16(FAR struct fat_format_s *fmt, FAR struct fat_var_s *var,
uint32_t navailsects)
{
-#ifdef CONFIG_HAVE_LONG_LONG
uint64_t denom;
uint64_t number;
-#else
- uint32_t denom;
- uint32_t number;
-#endif
Review Comment:
`mkfatfs_nfatsect16()` now declares `denom/number` as `uint64_t`, but later
`number = navailsects + ...` still performs the addition in 32-bit
(`navailsects` is `uint32_t`) and can wrap before assignment (e.g., near
`UINT32_MAX`). Please cast `navailsects` (and any shifted constants) to
`uint64_t` where `number` is computed so the intended 64-bit arithmetic is
actually used.
##########
fsutils/mkfatfs/configfat.c:
##########
@@ -156,25 +151,12 @@ mkfatfs_nfatsect12(FAR struct fat_format_s *fmt, FAR
struct fat_var_s *var,
* navailsects > 0x55555555 - 2 * clustersize
*/
-#ifndef CONFIG_HAVE_LONG_LONG
- if (navailsects <= (0x55555555 - (1 << (fmt->ff_clustshift + 1))))
- {
-#endif
-
denom = (fmt->ff_nfats << 1) + fmt->ff_nfats
+ (var->fv_sectorsize << (fmt->ff_clustshift + 1));
number = (navailsects << 1) + navailsects
+ (1 << (fmt->ff_clustshift + 2))
+ (1 << (fmt->ff_clustshift + 1));
Review Comment:
`mkfatfs_nfatsect12()` now uses `uint64_t` for `denom/number`, but `number`
is still computed with 32-bit arithmetic: `(navailsects << 1) + navailsects`
can wrap before being assigned to `uint64_t`. Cast `navailsects` (and any
shifted constants) to `uint64_t` before shifting/adding so the calculation is
actually done in 64-bit as intended.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
