Re: Nitpicking: unnecessary NULL-pointer check in pg_upgrade's controldata.c

Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> On Fri, Jun 26, 2015 at 10:21 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> I agree that the correct handling of this particular case is to mark it
>> as not-a-bug. We have better things to do.

> Well, I find that a disappointing conclusion, but I'm not going to
> spend a lot of time arguing against both of you. But, for what it's
> worth: it's not as if somebody is going to modify the code in that
> function to make output == NULL a plausible option, so I think the
> change could easily be justified on code clean-up grounds if nothing
> else. There's not much point calling fgets on a FILE unconditionally
> and then immediately thereafter allowing for the possibility that
> output might be NULL. That's not easing the work of anyone who might
> want to modify that code in the future; it just makes the code more
> confusing.

Well, if you find this to be good code cleanup on its own merits,
you have a commit bit, you can go commit it. I'm just saying that
Coverity is not a good judge of code readability and even less of
a judge of likely future changes. So we should not let it determine
whether we approve of "unnecessary" tests.

regards, tom lane