[tor-commits] [tor/master] Add return value and assert for null parameter to tor_munmap_file()

[nickm at torproject.org]

commit 389251eda902d16f3066bac6e8e5c0f596e3428f
Author: Andrea Shepard <andrea at torproject.org>
Date:   Tue Mar 18 10:47:17 2014 -0700

    Add return value and assert for null parameter to tor_munmap_file()
---
 src/common/compat.c |   54 ++++++++++++++++++++++++++++++++++++++++++---------
 src/common/compat.h |    2 +-
 2 files changed, 46 insertions(+), 10 deletions(-)

diff --git a/src/common/compat.c b/src/common/compat.c
index 8e2619f..b43af88 100644
--- a/src/common/compat.c
+++ b/src/common/compat.c
@@ -227,12 +227,27 @@ tor_mmap_file(const char *filename)
 
   return res;
 }
-/** Release storage held for a memory mapping. */
-void
+/** Release storage held for a memory mapping; returns 0 on success,
+ * or -1 on failure (and logs a warning). */
+int
 tor_munmap_file(tor_mmap_t *handle)
 {
-  munmap((char*)handle->data, handle->mapping_size);
-  tor_free(handle);
+  int res;
+
+  if (handle == NULL)
+    return 0;
+
+  res = munmap((char*)handle->data, handle->mapping_size);
+  if (res == 0) {
+    /* munmap() succeeded */
+    tor_free(handle);
+  } else {
+    log_warn(LD_FS, "Failed to munmap() in tor_munmap_file(): %s",
+             strerror(errno));
+    res = -1;
+  }
+
+  return res;
 }
 #elif defined(_WIN32)
 tor_mmap_t *
@@ -314,17 +329,27 @@ tor_mmap_file(const char *filename)
   tor_munmap_file(res);
   return NULL;
 }
-void
+
+/* Unmap the file, and return 0 for success or -1 for failure */
+int
 tor_munmap_file(tor_mmap_t *handle)
 {
-  if (handle->data)
+  tor_assert(handle != NULL);
+  if (handle->data) {
     /* This is an ugly cast, but without it, "data" in struct tor_mmap_t would
        have to be redefined as non-const. */
-    UnmapViewOfFile( (LPVOID) handle->data);
+    BOOL ok = UnmapViewOfFile( (LPVOID) handle->data);
+    if (!ok) {
+      log_warn(LD_FS, "Failed to UnmapViewOfFile() in tor_munmap_file(): %d",
+               (int)GetLastError());
+    }
+  }
 
   if (handle->mmap_handle != NULL)
     CloseHandle(handle->mmap_handle);
   tor_free(handle);
+
+  return 0;
 }
 #else
 tor_mmap_t *
@@ -340,13 +365,24 @@ tor_mmap_file(const char *filename)
   handle->size = st.st_size;
   return handle;
 }
-void
+
+/** Unmap the file mapped with tor_mmap_file(), and return 0 for success
+ * or -1 for failure.
+ */
+
+int
 tor_munmap_file(tor_mmap_t *handle)
 {
-  char *d = (char*)handle->data;
+  char *d = NULL;
+
+  tor_assert(handle != NULL);
+  d = (char*)handle->data;
   tor_free(d);
   memwipe(handle, 0, sizeof(tor_mmap_t));
   tor_free(handle);
+
+  /* Can't fail in this mmap()/munmap()-free case */
+  return 0;
 }
 #endif
 
diff --git a/src/common/compat.h b/src/common/compat.h
index 32effa5..c464464 100644
--- a/src/common/compat.h
+++ b/src/common/compat.h
@@ -292,7 +292,7 @@ typedef struct tor_mmap_t {
 } tor_mmap_t;
 
 tor_mmap_t *tor_mmap_file(const char *filename) ATTR_NONNULL((1));
-void tor_munmap_file(tor_mmap_t *handle) ATTR_NONNULL((1));
+int tor_munmap_file(tor_mmap_t *handle) ATTR_NONNULL((1));
 
 int tor_snprintf(char *str, size_t size, const char *format, ...)
   CHECK_PRINTF(3,4) ATTR_NONNULL((1,3));