tempfile: robustify cleanup handler
[gitweb.git] / tempfile.c
index c6740e562cb95e960365a34a63930077103c59e4..3348ad59dd3beb44e3057856edffaddea40a24c1 100644 (file)
 
 static struct tempfile *volatile tempfile_list;
 
-static void remove_tempfiles(int skip_fclose)
+static void remove_tempfiles(int in_signal_handler)
 {
        pid_t me = getpid();
+       struct tempfile *volatile p;
 
-       while (tempfile_list) {
-               if (tempfile_list->owner == me) {
-                       /* fclose() is not safe to call in a signal handler */
-                       if (skip_fclose)
-                               tempfile_list->fp = NULL;
-                       delete_tempfile(tempfile_list);
-               }
-               tempfile_list = tempfile_list->next;
+       for (p = tempfile_list; p; p = p->next) {
+               if (!is_tempfile_active(p) || p->owner != me)
+                       continue;
+
+               if (p->fd >= 0)
+                       close(p->fd);
+
+               if (in_signal_handler)
+                       unlink(p->filename.buf);
+               else
+                       unlink_or_warn(p->filename.buf);
+
+               p->active = 0;
        }
 }
 
@@ -95,8 +101,8 @@ static void prepare_tempfile_object(struct tempfile *tempfile)
                atexit(remove_tempfiles_on_exit);
        }
 
-       if (tempfile->active)
-               die("BUG: prepare_tempfile_object called for active object");
+       if (is_tempfile_active(tempfile))
+               BUG("prepare_tempfile_object called for active object");
        if (!tempfile->on_list) {
                /* Initialize *tempfile and add it to tempfile_list: */
                tempfile->fd = -1;
@@ -109,10 +115,22 @@ static void prepare_tempfile_object(struct tempfile *tempfile)
                tempfile->on_list = 1;
        } else if (tempfile->filename.len) {
                /* This shouldn't happen, but better safe than sorry. */
-               die("BUG: prepare_tempfile_object called for improperly-reset object");
+               BUG("prepare_tempfile_object called for improperly-reset object");
        }
 }
 
+static void activate_tempfile(struct tempfile *tempfile)
+{
+       tempfile->owner = getpid();
+       tempfile->active = 1;
+}
+
+static void deactivate_tempfile(struct tempfile *tempfile)
+{
+       tempfile->active = 0;
+       strbuf_reset(&tempfile->filename);
+}
+
 /* Make sure errno contains a meaningful value on error */
 int create_tempfile(struct tempfile *tempfile, const char *path)
 {
@@ -126,11 +144,10 @@ int create_tempfile(struct tempfile *tempfile, const char *path)
                tempfile->fd = open(tempfile->filename.buf,
                                    O_RDWR | O_CREAT | O_EXCL, 0666);
        if (tempfile->fd < 0) {
-               strbuf_reset(&tempfile->filename);
+               deactivate_tempfile(tempfile);
                return -1;
        }
-       tempfile->owner = getpid();
-       tempfile->active = 1;
+       activate_tempfile(tempfile);
        if (adjust_shared_perm(tempfile->filename.buf)) {
                int save_errno = errno;
                error("cannot fix permission bits on %s", tempfile->filename.buf);
@@ -145,8 +162,7 @@ void register_tempfile(struct tempfile *tempfile, const char *path)
 {
        prepare_tempfile_object(tempfile);
        strbuf_add_absolute_path(&tempfile->filename, path);
-       tempfile->owner = getpid();
-       tempfile->active = 1;
+       activate_tempfile(tempfile);
 }
 
 int mks_tempfile_sm(struct tempfile *tempfile,
@@ -157,11 +173,10 @@ int mks_tempfile_sm(struct tempfile *tempfile,
        strbuf_add_absolute_path(&tempfile->filename, template);
        tempfile->fd = git_mkstemps_mode(tempfile->filename.buf, suffixlen, mode);
        if (tempfile->fd < 0) {
-               strbuf_reset(&tempfile->filename);
+               deactivate_tempfile(tempfile);
                return -1;
        }
-       tempfile->owner = getpid();
-       tempfile->active = 1;
+       activate_tempfile(tempfile);
        return tempfile->fd;
 }
 
@@ -179,11 +194,10 @@ int mks_tempfile_tsm(struct tempfile *tempfile,
        strbuf_addf(&tempfile->filename, "%s/%s", tmpdir, template);
        tempfile->fd = git_mkstemps_mode(tempfile->filename.buf, suffixlen, mode);
        if (tempfile->fd < 0) {
-               strbuf_reset(&tempfile->filename);
+               deactivate_tempfile(tempfile);
                return -1;
        }
-       tempfile->owner = getpid();
-       tempfile->active = 1;
+       activate_tempfile(tempfile);
        return tempfile->fd;
 }
 
@@ -204,10 +218,10 @@ int xmks_tempfile_m(struct tempfile *tempfile, const char *template, int mode)
 
 FILE *fdopen_tempfile(struct tempfile *tempfile, const char *mode)
 {
-       if (!tempfile->active)
-               die("BUG: fdopen_tempfile() called for inactive object");
+       if (!is_tempfile_active(tempfile))
+               BUG("fdopen_tempfile() called for inactive object");
        if (tempfile->fp)
-               die("BUG: fdopen_tempfile() called for open object");
+               BUG("fdopen_tempfile() called for open object");
 
        tempfile->fp = fdopen(tempfile->fd, mode);
        return tempfile->fp;
@@ -215,34 +229,36 @@ FILE *fdopen_tempfile(struct tempfile *tempfile, const char *mode)
 
 const char *get_tempfile_path(struct tempfile *tempfile)
 {
-       if (!tempfile->active)
-               die("BUG: get_tempfile_path() called for inactive object");
+       if (!is_tempfile_active(tempfile))
+               BUG("get_tempfile_path() called for inactive object");
        return tempfile->filename.buf;
 }
 
 int get_tempfile_fd(struct tempfile *tempfile)
 {
-       if (!tempfile->active)
-               die("BUG: get_tempfile_fd() called for inactive object");
+       if (!is_tempfile_active(tempfile))
+               BUG("get_tempfile_fd() called for inactive object");
        return tempfile->fd;
 }
 
 FILE *get_tempfile_fp(struct tempfile *tempfile)
 {
-       if (!tempfile->active)
-               die("BUG: get_tempfile_fp() called for inactive object");
+       if (!is_tempfile_active(tempfile))
+               BUG("get_tempfile_fp() called for inactive object");
        return tempfile->fp;
 }
 
 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,18 +278,18 @@ int close_tempfile_gently(struct tempfile *tempfile)
 
 int reopen_tempfile(struct tempfile *tempfile)
 {
+       if (!is_tempfile_active(tempfile))
+               BUG("reopen_tempfile called for an inactive object");
        if (0 <= tempfile->fd)
-               die("BUG: reopen_tempfile called for an open object");
-       if (!tempfile->active)
-               die("BUG: reopen_tempfile called for an inactive object");
+               BUG("reopen_tempfile called for an open object");
        tempfile->fd = open(tempfile->filename.buf, O_WRONLY);
        return tempfile->fd;
 }
 
 int rename_tempfile(struct tempfile *tempfile, const char *path)
 {
-       if (!tempfile->active)
-               die("BUG: rename_tempfile called for inactive object");
+       if (!is_tempfile_active(tempfile))
+               BUG("rename_tempfile called for inactive object");
 
        if (close_tempfile_gently(tempfile)) {
                delete_tempfile(tempfile);
@@ -287,18 +303,16 @@ int rename_tempfile(struct tempfile *tempfile, const char *path)
                return -1;
        }
 
-       tempfile->active = 0;
-       strbuf_reset(&tempfile->filename);
+       deactivate_tempfile(tempfile);
        return 0;
 }
 
 void delete_tempfile(struct tempfile *tempfile)
 {
-       if (!tempfile->active)
+       if (!is_tempfile_active(tempfile))
                return;
 
        close_tempfile_gently(tempfile);
        unlink_or_warn(tempfile->filename.buf);
-       tempfile->active = 0;
-       strbuf_reset(&tempfile->filename);
+       deactivate_tempfile(tempfile);
 }