On 12/6/19 3:18 PM, Ranier Vilela wrote:
> Robert Haas wrote:
>> Maybe you want to post a few examples. It's hard to discuss in the abstract.
> I am working on the patch.
> I think this is a great example.
> I do not know if it is better to rename the local parameter, or if it should be renamed the global variable.
>
> line: 68
> var char **synchronous_commit
> backend/commands/subscriptioncmds.c
>
> global var declared here:
> /include/access/xact.h(82)
The local variables in subscriptioncmds.c named "synchronous_commit"
appear more times in that one file than the global variable appears
in total in the rest of the system, but that doesn't include other
references to the guc in code comments, in user facing strings, etc.
I think it is better to change this just in subscriptioncmds.c than
to change the global variable name everywhere else. I also tend to
agree with you that shadowing the global variable is bad practice.
> One question, is it better to submit the patch on this topic, or create a new one?
You appear to be planning to submit lots of patches about lots of
different shadowed variables. If you start a new thread for this
particular variable, it seems you'd probably do that again and again
for the other ones, and that might be tedious for readers of the
-hackers list who aren't interested. To start, I'd prefer to see
the patch on this thread.
--
Mark Dilger
