[Top][All Lists]
[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.
- Re: [PATCH] df: new option: --total (-c) to produce grand total (in the same way as du), Kamil Dudka, 2008/09/03
- Re: [PATCH] df: new option: --total (-c) to produce grand total (in the same way as du), Andreas Schwab, 2008/09/03
- Re: [PATCH] df: new option: --total (-c) to produce grand total (in the same way as du), Jim Meyering, 2008/09/03
- Re: [PATCH] df: new option: --total (-c) to produce grand total (in the same way as du), Andreas Schwab, 2008/09/03
- Re: [PATCH] df: new option: --total (-c) to produce grand total (in the same way as du), Jim Meyering, 2008/09/03
- Re: [PATCH] df: new option: --total (-c) to produce grand total (in the same way as du), Andreas Schwab, 2008/09/03
- Re: [PATCH] df: new option: --total (-c) to produce grand total (in the same way as du), Jim Meyering, 2008/09/03
- Re: [PATCH] df: new option: --total (-c) to produce grand total (in the same way as du), Andreas Schwab, 2008/09/03
Re: [PATCH] df: new option: --total (-c) to produce grand total (in the same way as du), Kamil Dudka, 2008/09/03