bug-gnulib
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] df: new option: --total (-c) to produce grand total (in the


From: Jim Meyering
Subject: Re: [PATCH] df: new option: --total (-c) to produce grand total (in the same way as du)
Date: Wed, 03 Sep 2008 16:20:35 +0200

address@hidden (Eric Blake) wrote:
>> > I'm referring to the use of the very same variables that are used in the
>> > patch.  If those are not pure boolean then you have a bug anyway.
>>
>> Here are some of the changes needed to protect against the substandard
>> "bool" problem we're talking about.  Some of the changes (& => &&)
>> are unconditional improvements, imho.  However, the others are not.
>> Before I start polluting the code with those "fixes", I'd like
>> confirmation that this is a problem in more than just theory.
>
> My understanding of gnulib's stdbool is that _if_ you guarantee

I agree.  The question is how best to *maintain* the precondition in
the face of future development.  This is sufficiently subtle, and the
problem (if a violation occurs) sufficiently hard to reproduce (only on
aging proprietary systems) that I do see some value in making the uses
immune to future accidental violation of the precondition.

On the other hand, I prefer not to pander to substandard systems,
so I'm torn.  Maybe I won't apply it after all.
Other opinions welcome.

> the precondition of only ever assigning 0 or 1 to a bool variable,
> then all other uses of bool work as in C99 without having to
> uglify code (& can be more efficient than &&, you don't need to
> use !! to convert a bool back into a 0/1 value, etc).  The portability
> warning in stdbool.in.h is telling users to avoid assigning
> non-zero values of anything other than 1 into a bool, so that you
> don't have to worry about the use of that bool.
>
>> @@ -185,11 +185,11 @@ print_header (void)
>>        divisible_by_1000 = q1000 % 1000 == 0;  q1000 /= 1000;
>>        divisible_by_1024 = q1024 % 1024 == 0;  q1024 /= 1024;
>
> Here, you are assigning a bool variable by using the ==
> operator, which guarantees 0/1...
>
>>      }
>> -      while (divisible_by_1000 & divisible_by_1024);
>> +      while (divisible_by_1000 && divisible_by_1024);
>>
>> -      if (divisible_by_1000 < divisible_by_1024)
>> +      if (!!divisible_by_1000 < !!divisible_by_1024)
>
> ...so patches like this should NOT be applied to coreutils,
> because they add no value.  The portability bugs that
> you must worry about when using gnulib's bool are not
> in the use of bool, but in the assignment to bool.  That is,
>
> bool foo = a & mask;
>
> is wrong, you should use one of
>
> bool bar = !!(a & mask);
> bool baz = (a & mask) == 0;
>
> But your proposed patch did not correct any bugs in
> meeting the preconditions of bool.




reply via email to

[Prev in Thread] Current Thread [Next in Thread]