GNU bug report logs - #48960
stat v8.30 - device number in decimal shown as 16bit number instead of to converted 8bit

Previous Next

Full log

Message #25 received at 48960-done <at> debbugs.gnu.org (full text, mbox):

From: Pádraig Brady <P <at> draigBrady.com>
To: Paul Eggert <eggert <at> cs.ucla.edu>, wolfgang.rohm <at> arcor.de,
 48960-done <at> debbugs.gnu.org
Subject: Re: bug#48960: stat v8.30 - device number in decimal shown as 16bit
 number instead of to converted 8bit
Date: Mon, 21 Jun 2021 12:07:01 +0100

On 20/06/2021 22:38, Paul Eggert wrote:
> Thanks for writing that patch. One minor note:
> 
> On 6/20/21 7:21 AM, Pádraig Brady wrote:
> 
>> +                                    (to_uchar (mod_char) << CHAR_BIT)
>> +                                     + to_uchar (fmt_char),
> 
> Neither mod_char nor fmt_char can be negative (this is guaranteed by the
> C standard since all the relevant constants are in the basic character
> set) so the to_uchar calls are unnecessary.
> 
> Also, this code assumes that 2 * CHAR_BIT <= MIN (INT_WIDTH,
> UINT_WIDTH), something that POSIX requires but the C standard does not;
> it'd be a bit safer (if pedantic) to add 'verify (2 * CHAR_BIT <= MIN
> (INT_WIDTH, UINT_WIDTH));'.
> 
> I'd also change print_stat's arg from unsigned int to int; if you did
> that, you could change the above 'MIN (INT_WIDTH, UINT_WIDTH)' to plain
> 'INT_WIDTH'. (These days we're negative on unsigned types anyway, for
> all the usual reasons....)
> 
> Better yet, pass two char args to print_stat instead of a single int
> portmanteau.

Yes two separate char arguments is cleaner.
I thought there was some reason for the existing unsigned int usage,
but looking I see it was added for commit db42ae78 (to support :X formats),
but then became unneeded with commit c7375c23 (to instead support %.9X formats).

I've pushed both commits now, so marking this as done,

thanks for the review!
Pádraig

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.