[EMAIL PROTECTED] (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.