On 2016-03-02 21:48:16 -0500, Peter Eisentraut wrote: > After reviewing this thread and relevant internet lore, I think this > might be the wrong way to address this problem. It is in general not > guaranteed in C that a Boolean-sounding function or macro returns 0 or > 1. Prime examples are ctype.h functions such as isupper(). This is > normally not a problem because built-in conditionals such as if, &&, || > handle this just fine. So code like > > - Assert(!create || !!txn); > + Assert(!create || txn != NULL); > > is arguably silly either way. There is no risk in writing just > > Assert(!create || txn);
Yes, obviously. I doubt that anyone misunderstood that. I personally find the !! easier to read when contrasting to a negated value (as in the above assert). > The problem only happens if you compare two "Boolean" values directly > with each other; That's not correct. It also happen if you compare an expression with a stored version, i.e. only one side is a 'proper bool'. > A quick look through the code based on the provided patch shows that > approximately the only place affected by this is > > if (isLeaf != GinPageIsLeaf(page) || isData != GinPageIsData(page)) > elog(ERROR, "right sibling of GIN page is of different type"); > > and that's not actually a problem because isLeaf and isData are earlier > populated by the same macros. It would still be worth fixing, but a > localized fix seems better. That's maybe the only place where we compare stored booleans to expressions, but it's definitely not the only place where some expression is stored in a boolean variable. And in all those cases there's an int32 (or whatever type the expression has) to bool (usually 1byte char). That's definitely problematic. > But the lore on the internet casts some doubt on that: There is no > guarantee that bool is 1 byte, that bool can be passed around like char, > or even that bool arrays are laid out like char arrays. Maybe this all > works out okay, just like it has worked out so far that int is 4 bytes, > but we don't know enough about it. We could probably add some configure > tests around that. So? > My proposal on this particular patch is to do nothing. The stdbool > issues should be looked into, for the sake of Windows and other > future-proofness. But that will likely be an entirely different patch. That seems to entirely miss the part of this discussion dealing with high bits set and such? Greetings, Andres Freund -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers