From: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
Date: Sun, 04 Mar 2007 17:08:49 GMT
Subject: Re: [PATCH 1/6] Fix some "printf format" warnings.
Message-ID: <45EAFD21.6010002@ramsay1.demon.co.uk>
In-Reply-To: <7v4pp29eok.fsf@assigned-by-dhcp.cox.net>

Hi Junio,

Junio C Hamano wrote:
> 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