Skip to content

Commit

Permalink
tempfile: avoid "ferror | fclose" trick
Browse files Browse the repository at this point in the history
The current code wants to record an error condition from
either ferror() or fclose(), but makes sure that we always
call both functions. So it can't use logical-OR "||", which
would short-circuit when ferror() is true. Instead, it uses
bitwise-OR "|" to evaluate both functions and set one or
more bits in the "err" flag if they reported a failure.

Unlike logical-OR, though, bitwise-OR does not introduce a
sequence point, and the order of evaluation for its operands
is unspecified. So a compiler would be free to generate code
which calls fclose() first, and then ferror() on the
now-freed filehandle.

There's no indication that this has happened in practice,
but let's write it out in a way that follows the standard.

Noticed-by: Andreas Schwab <schwab@linux-m68k.org>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
  • Loading branch information
peff authored and gitster committed Feb 16, 2017
1 parent c3808ca commit 0838cbc
Showing 1 changed file with 2 additions and 6 deletions.
8 changes: 2 additions & 6 deletions tempfile.c
Original file line number Diff line number Diff line change
Expand Up @@ -247,12 +247,8 @@ int close_tempfile(struct tempfile *tempfile)
tempfile->fd = -1;
if (fp) {
tempfile->fp = NULL;

/*
* Note: no short-circuiting here; we want to fclose()
* in any case!
*/
err = ferror(fp) | fclose(fp);
err = ferror(fp);
err |= fclose(fp);
} else {
err = close(fd);
}
Expand Down

0 comments on commit 0838cbc

Please sign in to comment.