Re: fix for new SUSET GUC variables

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Aizaz Ahmed <aahmed(at)redhat(dot)com>
Cc: Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us>, PostgreSQL-patches <pgsql-patches(at)postgresql(dot)org>, Fernando Nasser <fnasser(at)redhat(dot)com>
Subject: Re: fix for new SUSET GUC variables
Date: 2003-07-14 20:33:14
Message-ID: 13892.1058214794@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-patches

Aizaz Ahmed <aahmed(at)redhat(dot)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

In response to

Responses

Browse pgsql-patches by date

  From Date Subject
Next Message Tom Lane 2003-07-14 20:35:06 Re: typo in src/include/utils/array.h
Previous Message Jon Jensen 2003-07-11 16:25:48 Revised sslmode patch