commit abdf1878a3f8fe366d8bb7f649127880bdbdf82d Author: Andrea Shepard andrea@torproject.org Date: Tue Mar 18 17:52:31 2014 -0700

Always check returns from unlink() --- src/common/util.c | 9 ++++++++- src/or/config.c | 5 ++++- src/or/hibernate.c | 10 +++++++++- src/or/main.c | 17 +++++++++++++---- src/or/networkstatus.c | 21 +++++++++++++++++---- src/or/statefile.c | 10 ++++++++-- 6 files changed, 59 insertions(+), 13 deletions(-)

diff --git a/src/common/util.c b/src/common/util.c index 3c2f664..c4f8c88 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 0da4877..5b590f2 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 607dec8..bbda842 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 7294c89..3d5ce10 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 49478a7..74c4ca4 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 8ab0476..2251f25 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 "