Re: minor smgr code cleanup - Mailing list pgsql-patches

From Tom Lane
Subject Re: minor smgr code cleanup
Date
Msg-id 29199.1073348327@sss.pgh.pa.us
Whole thread Raw
In response to minor smgr code cleanup  (Neil Conway <neilc@samurai.com>)
Responses Re: minor smgr code cleanup  (Neil Conway <neilc@samurai.com>)
List pgsql-patches
Neil Conway <neilc@samurai.com> writes:
>   bool
>   IsReservedName(const char *name)
>   {
> !     /* ugly coding for speed */
> !     return (name[0] == 'p' &&
> !             name[1] == 'g' &&
> !             name[2] == '_');
>   }
> --- 160,178 ----
>   bool
>   IsReservedName(const char *name)
>   {
> !     return strncmp(name, "pg_", 3);
>   }

This change is actually wrong (backwards), no?  You want a true result
on equality.

In any case I don't think this is a step forward in readability, and it
also poses a portability risk.  You should always write such tests as

    return strncmp(name, "pg_", 3) == 0;

(or != 0 as appropriate).  Pretending that the result of strcmp is a
bool is a type pun, and one that can rise up to bite you.  In the case
at hand, strncmp is allowed to return (say) 256 to indicate a nonzero
result --- which would be lost when the value is squeezed into a bool
(char).  See the archives; we've had at least one bug of this ilk.

            regards, tom lane

pgsql-patches by date:

Previous
From: Kurt Roeckx
Date:
Subject: Re: remove obsolete NULL casts
Next
From: Tom Lane
Date:
Subject: List rewrite (was Re: minor smgr code cleanup)