Re: fix for new SUSET GUC variables - Mailing list pgsql-patches

From Tom Lane
Subject Re: fix for new SUSET GUC variables
Date
Msg-id 13892.1058214794@sss.pgh.pa.us
Whole thread Raw
In response to Re: fix for new SUSET GUC variables  (Aizaz Ahmed <aahmed@redhat.com>)
Responses Re: fix for new SUSET GUC variables  (Bruce Momjian <pgman@candle.pha.pa.us>)
List pgsql-patches
Aizaz Ahmed <aahmed@redhat.com> writes:
> I believe when updating the GucContext enum, it is also necessary to
> update the GucContext_names [] in backend/utils/misc/help_config.c.
> The need to do this was supposed to be added as a comment to the guc.h
> file, right about where GucContext is defined, but it seems as if that
> part of the patch was not applied.

I did not include that comment because it seemed misleading (that array
is far from the only place that has to be adjusted when adding a new
GucContext value) as well as distracting (GucContext_names is not
exactly the most important thing to know about when trying to understand
GucContext).

We don't normally try to enumerate in comments all the places you'd need
to change when adding to an enum or other widely-used definition.  You're
supposed to find them by searching the source code for references to the
existing values.  Depending on comments for that sort of thing is far
too error-prone --- you can just about guarantee that the comment will
fail to track new uses.

The reason Bruce's patch broke the array is that he applied an old patch
without sufficient checking on what had changed in the meantime.
If the comment had been there, it would not have saved him from this
error, since I doubt it would have occurred to him to look for such a
comment.

            regards, tom lane

pgsql-patches by date:

Previous
From: Jon Jensen
Date:
Subject: Revised sslmode patch
Next
From: Tom Lane
Date:
Subject: Re: typo in src/include/utils/array.h