On 04/11, Jeff King wrote:
> On Sun, Apr 07, 2019 at 07:47:51PM +0100, Thomas Gummerer wrote:
>
> > struct stat_data and struct cache_time both use unsigned ints for all
> > their members. However the format string for 'git ls-files --debug'
> > currently uses %d for formatting these numbers. This means that we
> > potentially print these values incorrectly if they are greater than
> > INT_MAX.
> >
> > This has been the case since the --debug option was introduced in 'git
> > ls-files' in 8497421715 ("ls-files: learn a debugging dump format",
> > 2010-07-31).
>
> I didn't see any comment on this, but it seems like it must be obviously
> correct, since as you note we do define those fields as unsigned. I'm
> really surprised that -Wformat doesn't catch this, though. I wonder why.
Good point. A bit of digging led me to -Wformat-signedness, which
should catch this. This turns up a lot of errors in our codebase. I
didn't go through to see how many of them are actual errors, and how
many are false-positives though.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65446 describes how the
option can lead to false positives, e.g.
printf ("%u\n", unsigned_short);
might turn up an error. From a quick test this seems to work
correctly with gcc 8.2.1 that I have on my machine though, so the
issue might be fixed in newer gcc version, even though that bug report
is still marked as new.
Maybe it's worth going through the warnings at some point to see if it
would be possible to turn -Wformat-signedness on.
