Always check returns from unlink()

6 files changed, 59 insertions, 13 deletions

diff --git a/src/common/util.c b/src/common/util.c
index 3c2f6643a..c4f8c8802 100644
--- a/src/common/util.c
+++ b/src/common/util.c
@@ -2141,6 +2141,7 @@ static int
 finish_writing_to_file_impl(open_file_t *file_data, int abort_write)
 {
   int r = 0;
+
   tor_assert(file_data && file_data->filename);
   if (file_data->stdio_file) {
     if (fclose(file_data->stdio_file)) {
@@ -2157,7 +2158,13 @@ finish_writing_to_file_impl(open_file_t *file_data, int abort_write)
   if (file_data->rename_on_close) {
     tor_assert(file_data->tempname && file_data->filename);
     if (abort_write) {
-      unlink(file_data->tempname);
+      int res = unlink(file_data->tempname);
+      if (res != 0) {
+        /* We couldn't unlink and we'll leave a mess behind */
+        log_warn(LD_FS, "Failed to unlink %s: %s",
+                 file_data->tempname, strerror(errno));
+        r = -1;
+      }
     } else {
       tor_assert(strcmp(file_data->filename, file_data->tempname));
       if (replace_file(file_data->tempname, file_data->filename)) {
diff --git a/src/or/config.c b/src/or/config.c
index 0da4877a5..5b590f21c 100644
--- a/src/or/config.c
+++ b/src/or/config.c
@@ -6464,7 +6464,10 @@ remove_file_if_very_old(const char *fname, time_t now)
     format_local_iso_time(buf, st.st_mtime);
     log_notice(LD_GENERAL, "Obsolete file %s hasn't been modified since %s. "
                "Removing it.", fname, buf);
-    unlink(fname);
+    if (unlink(fname) != 0) {
+      log_warn(LD_FS, "Failed to unlink %s: %s",
+               fname, strerror(errno));
+    }
   }
 }
 
diff --git a/src/or/hibernate.c b/src/or/hibernate.c
index 607dec8cd..bbda8424f 100644
--- a/src/or/hibernate.c
+++ b/src/or/hibernate.c
@@ -648,7 +648,15 @@ read_bandwidth_usage(void)
 
   {
     char *fname = get_datadir_fname("bw_accounting");
-    unlink(fname);
+    int res;
+
+    res = unlink(fname);
+    if (res != 0) {
+      log_warn(LD_FS,
+               "Failed to unlink %s: %s",
+               fname, strerror(errno));
+    }
+
     tor_free(fname);
   }
 
diff --git a/src/or/main.c b/src/or/main.c
index 7294c8955..3d5ce10cf 100644
--- a/src/or/main.c
+++ b/src/or/main.c
@@ -2574,10 +2574,19 @@ tor_cleanup(void)
     time_t now = time(NULL);
     /* Remove our pid file. We don't care if there was an error when we
      * unlink, nothing we could do about it anyways. */
-    if (options->PidFile)
-      unlink(options->PidFile);
-    if (options->ControlPortWriteToFile)
-      unlink(options->ControlPortWriteToFile);
+    if (options->PidFile) {
+      if (unlink(options->PidFile) != 0) {
+        log_warn(LD_FS, "Couldn't unlink pid file %s: %s",
+                 options->PidFile, strerror(errno));
+      }
+    }
+    if (options->ControlPortWriteToFile) {
+      if (unlink(options->ControlPortWriteToFile) != 0) {
+        log_warn(LD_FS, "Couldn't unlink control port file %s: %s",
+                 options->ControlPortWriteToFile,
+                 strerror(errno));
+      }
+    }
     if (accounting_is_enabled(options))
       accounting_record_bandwidth_usage(now, get_or_state());
     or_state_mark_dirty(get_or_state(), 0); /* force an immediate save. */
diff --git a/src/or/networkstatus.c b/src/or/networkstatus.c
index 49478a734..74c4ca45a 100644
--- a/src/or/networkstatus.c
+++ b/src/or/networkstatus.c
@@ -1254,7 +1254,11 @@ networkstatus_set_current_consensus(const char *consensus,
         /* Even if we had enough signatures, we'd never use this as the
          * latest consensus. */
         if (was_waiting_for_certs && from_cache)
-          unlink(unverified_fname);
+          if (unlink(unverified_fname) != 0) {
+            log_warn(LD_FS,
+                     "Failed to unlink %s: %s",
+                     unverified_fname, strerror(errno));
+          }
       }
       goto done;
     } else {
@@ -1264,8 +1268,13 @@ networkstatus_set_current_consensus(const char *consensus,
                  "consensus");
         result = -2;
       }
-      if (was_waiting_for_certs && (r < -1) && from_cache)
-        unlink(unverified_fname);
+      if (was_waiting_for_certs && (r < -1) && from_cache) {
+        if (unlink(unverified_fname) != 0) {
+            log_warn(LD_FS,
+                     "Failed to unlink %s: %s",
+                     unverified_fname, strerror(errno));
+        }
+      }
       goto done;
     }
   }
@@ -1313,7 +1322,11 @@ networkstatus_set_current_consensus(const char *consensus,
       waiting->body = NULL;
     waiting->set_at = 0;
     waiting->dl_failed = 0;
-    unlink(unverified_fname);
+    if (unlink(unverified_fname) != 0) {
+      log_warn(LD_FS,
+               "Failed to unlink %s: %s",
+               unverified_fname, strerror(errno));
+    }
   }
 
   /* Reset the failure count only if this consensus is actually valid. */
diff --git a/src/or/statefile.c b/src/or/statefile.c
index 8ab04763d..2251f25e9 100644
--- a/src/or/statefile.c
+++ b/src/or/statefile.c
@@ -260,7 +260,7 @@ or_state_set(or_state_t *new_state)
 static void
 or_state_save_broken(char *fname)
 {
-  int i;
+  int i, res;
   file_status_t status;
   char *fname2 = NULL;
   for (i = 0; i < 100; ++i) {
@@ -274,7 +274,13 @@ or_state_save_broken(char *fname)
     log_warn(LD_BUG, "Unable to parse state in \"%s\"; too many saved bad "
              "state files to move aside. Discarding the old state file.",
              fname);
-    unlink(fname);
+    res = unlink(fname);
+    if (res != 0) {
+      log_warn(LD_FS,
+               "Also couldn't discard old state file \"%s\" because "
+               "unlink() failed: %s",
+               fname, strerror(errno));
+    }
   } else {
     log_warn(LD_BUG, "Unable to parse state in \"%s\". Moving it aside "
              "to \"%s\".  This could be a bug in Tor; please tell "