On mån, 2012-03-26 at 15:53 -0400, Tom Lane wrote:
> Peter Eisentraut <peter_e@gmx.net> writes:
> > On mån, 2012-03-26 at 15:15 -0400, Tom Lane wrote:
> >> I also do not think it does anything for readability for this call
> >> of read_info() to be unexpectedly unlike all the others.
>
> > I do not think that it is good code quality to assign something to a
> > variable and then assign something different to a variable later in the
> > same function.
>
> Well, that's a totally different issue, because if we had used a
> different variable for the other purpose, this assignment would
> still be dead and coverity would still be whinging about it, no?
Let's look at this differently. If this code were written from scratch
now, it might have turned out like this:
Form_pg_sequence old_seq, seq;
...
old_seq = read_info(elm, seq_rel, &buf);
...
seq = (Form_pg_sequence) GETSTRUCT(tuple);
But that gets a complaint from gcc:
sequence.c:248:19: error: variable ‘old_seq’ set but not used [-Werror=unused-but-set-variable]
So when faced with this, what is the right fix? (Probably not assigning
the useless return value to some other variable used for a different
purpose.)
> The problem that I have with this change (and the similar ones you've
> made elsewhere) is really that it's only chance that the code isn't
> fetching anything from the result of read_info.
What other changes are you referring to? I don't recall any similar
ones and don't find any in the logs.