Re: [PATCH] pack-revindex.c: don't close unopened file descriptors

[Jeff King <peff@peff.net>]

On Fri, Feb 26, 2021 at 11:31:02AM -0500, Taylor Blau wrote:

> When opening a reverse index, load_revindex_from_disk() jumps to the
> 'cleanup' label in case something goes wrong: the reverse index had the
> wrong size, an unrecognized version, or similar.
> 
> It also jumps to this label when the reverse index couldn't be opened in
> the first place, which will cause an error with the unguarded close()
> call in the label.
> 
> Guard this call with "if (fd >= 0)" to make sure that we have a valid
> file descriptor to close before attempting to close it.

Makes sense. Running close(-1) wasn't really hurting much in practice,
but it's cleaner not to do so.

This would go on top of tb/pack-revindex-on-disk.

> Dscho mentioned this to me privately when reviewing Coverity results for
> -rc0. This one is legitimate, and the fix is easy enough, too.

I'm excited that we might get Coverity results again. There were a lot
of false positives, but I found its signal-to-noise ratio higher than
almost every other static analysis tool I've looked at.

-Peff