Memo on coding practices: strcmp() does not yield bool

I've occasionally griped that I do not like the coding practice of
writing
if (strcmp(foo, bar))
to mean
if (strcmp(foo, bar) != 0)
or its inverse
if (!strcmp(foo, bar))
to mean
if (strcmp(foo, bar) == 0)

My past objection to this has been purely stylistic: it's too easy
to read these constructs backwards, eg to think "!strcmp()" means
"not equal". However, I've now had my nose rubbed in the fact that
this habit is actually dangerous.

Up till just now, ruleutils.c contained code like this:

bool tell_as = FALSE;

/* Check if we must say AS ... */
if (!IsA(tle->expr, Var))
tell_as = strcmp(tle->resdom->resname, "?column?");

/* more code... */

if (tell_as)
/* do something */

This is subtly wrong, because it will work as intended on many
platforms. But on some platforms, strcmp is capable of yielding
values that are not 0 but whose low 8 bits are all 0. Stuff that
into a char-sized "bool" variable, and all of a sudden it's zero,
reversing the intended behavior of the test.

Correct, portable coding of course is

tell_as = strcmp(tle->resdom->resname, "?column?") != 0;

This error would not have happened if the author of this code had
been in the habit of regarding strcmp's result as something to compare
against 0, rather than as equivalent to a boolean value. So, I assert
that the above-mentioned coding practice is dangerous, because it can
lead you to do things that aren't portable.

I'm not planning to engage in a wholesale search-and-destroy mission
for misuses of strcmp and friends just at the moment, but maybe someone
should --- we may have comparable portability bugs elsewhere. In any
case I suggest we avoid this coding practice in future.

regards, tom lane