| From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
|---|---|
| To: | Teodor Sigaev <teodor(at)sigaev(dot)ru> |
| Cc: | Robert Haas <robertmhaas(at)gmail(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, Michael Paquier <michael(dot)paquier(at)gmail(dot)com>, Yury Zhuravlev <u(dot)zhuravlev(at)postgrespro(dot)ru>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, PostgreSQL mailing lists <pgsql-hackers(at)postgresql(dot)org> |
| Subject: | Re: GinPageIs* don't actually return a boolean |
| Date: | 2016-02-12 16:29:44 |
| Message-ID: | 29927.1455294584@sss.pgh.pa.us |
| Views: | Raw Message | Whole Thread | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Teodor Sigaev <teodor(at)sigaev(dot)ru> writes:
> One more option for patch:
> #define GinPageIsLeaf(page) ((bool)(GinPageGetOpaque(page)->flags & GIN_LEAF))
I think that's a seriously bad coding pattern to adopt, because it would
work for some people but not others if the flag bit is to the left of the
rightmost byte. We should standardize on the "((var & FLAG) != 0)"
pattern, which works reliably in all cases.
The pattern "(!!(var & FLAG))" would work too, but I dislike it because
it is not "say what you mean" but more of a cute coding trick to save a
keystroke or two. People who aren't longtime C coders would have to stop
and think about what it does.
(I'd expect reasonable compilers to generate pretty much the same code
in any of these cases, so that aspect of it shouldn't be an issue.)
regards, tom lane
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Andres Freund | 2016-02-12 16:35:26 | Re: GinPageIs* don't actually return a boolean |
| Previous Message | Christian Ullrich | 2016-02-12 16:24:24 | Crash with old Windows on new CPU |