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

Robert Haas <robertmhaas@gmail.com> writes:
> On Fri, Jun 26, 2015 at 10:21 AM, Tom Lane <tgl@sss.pgh.pa.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