Re: [PATCH 1/6] Fix some "printf format" warnings.
- From
Ramsay Jones <ramsay@ramsay1.demon.co.uk>
- Date
- Mar 4, 2007, 17:08 UTC
- Message-ID
- <45EAFD21.6010002@ramsay1.demon.co.uk>
- In-Reply-To
- <7v4pp29eok.fsf@assigned-by-dhcp.cox.net>
Hi Junio,
Junio C Hamano wrote:
Show 25 quoted lines
> Ramsay Jones <ramsay@ramsay1.demon.co.uk> writes:
>> diff --git a/builtin-ls-files.c b/builtin-ls-files.c
>> index ac89eb2..5c990a5 100644
>> --- a/builtin-ls-files.c
>> +++ b/builtin-ls-files.c
>> @@ -193,7 +193,7 @@ static void show_ce_entry(const char *tag, struct cache_entry *ce)
>> putchar(line_terminator);
>> }
>> else {
>> - printf("%s%06o %s %d\t",
>> + printf("%s%06lo %s %d\t",
>> tag,
>> ntohl(ce->ce_mode),
>> abbrev ? find_unique_abbrev(ce->sha1,abbrev)
>
> I think the issue is ntohl() returns uint32_t, and this did not
> surface as an issue so far only because that type happens to be
> defined as 'unsigned int' on many systems. Changing %o to %lo
> is shifting the breakage to other systems, isn't it?
>
> I think we should do this instead:
>
> printf("%s%06o %s %d\t", tag, (unsigned) ntohl(ce->ce_mode), ...
>
> Oops, yes you are right. (cygwin typedef's uint32_t as unsigned long.)
However, I would hate to add all those casts! Casts are not always evil, but should be avoided if possible. Having said that, I don't see another solution ... Hmmm, I dunno, your call ;-)
All the best,
Ramsay Jones