search

littlefs-project / littlefs

A little fail-safe filesystem designed for microcontrollers
BSD 3-Clause "New" or "Revised" License
4.9k stars 770 forks source link

Wrong printf specifiers #982

Open stefano-zanotti opened 1 month ago

stefano-zanotti commented 1 month ago

When LFS_TRACE is enabled, some printf-like calls are compiled. Some of the %-specifiers are wrong (they do not match the actual type of the arguments), which raises compiler warnings, possibly escalated to errors. In two cases (see below), the specifier should be changed. In another one, a cast is needed.

In lfs.c:5889: ".block_cycles=%"PRIu32 --> PRIi32

In lfs.c:5919: ".block_cycles=%"PRIu32 --> PRIi32

In lfs.c:6078: flags --> (unsigned)flags

geky commented 1 week ago

Hi @stefano-zanotti, thanks for creating an issue.

This would be good to fix.

Out of curiousity, what compiler are you using? Compiling littlefs with -DLFS_YES_TRACE unfortunately doesn't report these warnings with our current makefile.

geky commented 1 week ago

Compiling littlefs with -DLFS_YES_TRACE unfortunately doesn't report these warnings with our current makefile.

In hindsight what PRI expands to depends on the exact architecture. Given how printf/PRI works, I don't think it's going to be possible for us to prevent these sort of mistakes in CI...

geky commented 1 week ago

I gone ahead and created https://github.com/littlefs-project/littlefs/pull/997 to fix this. Will bring it in on the next patch, though this is low priority.

I had forgotten printf accepts %i, but I changed these to PRId32 for consistency.

Let me know if you see any other uncaught format warnings.

stefano-zanotti commented 1 week ago

I'm using GCC, with these flags (among others) -Werror=format-extra-args -Werror=format-overflow=2 -Werror=format-signedness -Werror=format-truncation=2 -Werror=format=2

(According to the docs, the latter implies -Werror=format -Werror=format-nonliteral -Werror=format-security -Werror=format-y2k)

The specific issue reported here is raised by "-Werror=format=2"; GCC reports something like: error: format '%lu' expects argument of type 'long unsigned int', but argument 14 has type 'int32_t' {aka 'long int'} [-Werror=format=]

Signedness problems should be evident on every platform, though it's true that number-of-bits errors might be missed by CI depending on the target architecture. Anyway, not catching these automatically is not really a big deal, I think. It's mostly to get a warning-free compilation, so that actual problems are not drowned out by a large number of minor warnings.

It's very possible that I missed some other cases (as you've already found), because of the specific LFS configuration flags I'm using. I'll let you know if something else comes up in the future.