The tempfile functions all take pointers to tempfile
objects, but do not check whether the argument is NULL.
This isn't a big deal in practice, since the lifetime of any
tempfile object is defined to last for the whole program. So
even if we try to call delete_tempfile() on an
already-deleted tempfile, our "active" check will tell us
that it's a noop.
In preparation for transitioning to a new system that
loosens the "tempfile objects can never be freed" rule,
let's tighten up our active checks:
1. A NULL pointer is now defined as "inactive" (so it will
BUG for most functions, but works as a silent noop for
things like delete_tempfile).
2. Functions should always do the "active" check before
looking at any of the struct fields.
Signed-off-by: Jeff King <peff@xxxxxxxx>
---
tempfile.c | 12 +++++++-----
tempfile.h | 2 +-
2 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/tempfile.c b/tempfile.c
index 964c66d504..861f817133 100644
--- a/tempfile.c
+++ b/tempfile.c
@@ -236,13 +236,15 @@ FILE *get_tempfile_fp(struct tempfile *tempfile)
int close_tempfile_gently(struct tempfile *tempfile)
{
- int fd = tempfile->fd;
- FILE *fp = tempfile->fp;
+ int fd;
+ FILE *fp;
int err;
- if (fd < 0)
+ if (!is_tempfile_active(tempfile) || tempfile->fd < 0)
return 0;
+ fd = tempfile->fd;
+ fp = tempfile->fp;
tempfile->fd = -1;
if (fp) {
tempfile->fp = NULL;
@@ -262,10 +264,10 @@ int close_tempfile_gently(struct tempfile *tempfile)
int reopen_tempfile(struct tempfile *tempfile)
{
- if (0 <= tempfile->fd)
- die("BUG: reopen_tempfile called for an open object");
if (!is_tempfile_active(tempfile))
die("BUG: reopen_tempfile called for an inactive object");
+ if (0 <= tempfile->fd)
+ die("BUG: reopen_tempfile called for an open object");
tempfile->fd = open(tempfile->filename.buf, O_WRONLY);
return tempfile->fd;
}
diff --git a/tempfile.h b/tempfile.h
index d854dcdd3e..d30663182d 100644
--- a/tempfile.h
+++ b/tempfile.h
@@ -211,7 +211,7 @@ extern FILE *fdopen_tempfile(struct tempfile *tempfile, const char *mode);
static inline int is_tempfile_active(struct tempfile *tempfile)
{
- return tempfile->active;
+ return tempfile && tempfile->active;
}
/*
--
2.14.1.721.gc5bc1565f1
