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: Eric Blake
Subject: Re: [PATCH] df: new option: --total (-c) to produce grand total (in the same way as du)
Date: Wed, 03 Sep 2008 14:09:44 +0000

> >
> > 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
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.

-- 
Eric Blake




reply via email to

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