libnsm.a: Factor atomic write code out of nsm_get_state()

We're about to use the same logic (mktemp, write, rename) for
other new purposes, so pull it out into its own function.

This change also addresses a latent bug: O_TRUNC is now used when
creating the temporary file.  This eliminates the possibility of
getting stale data in the temp file, if somehow a previous "atomic
write" was interrupted and didn't remove the temporary file.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>

diff --git a/support/nsm/file.c b/support/nsm/file.c
index fc3241a1a4d9a10d128d22f3d352dcbc2445863d..10769d9df9e87fdc4ff7302606d7fa59ea818e88 100644 (file)
--- a/support/nsm/file.c
+++ b/support/nsm/file.c
@@ -195,6 +195,94 @@ nsm_make_pathname(const char *directory)
        return path;
 }
 
+/*
+ * Returns a dynamically allocated, '\0'-terminated buffer
+ * containing an appropriate pathname, or NULL if an error
+ * occurs.  Caller must free the returned result with free(3).
+ */
+__attribute_malloc__
+static char *
+nsm_make_temp_pathname(const char *pathname)
+{
+       size_t size;
+       char *path;
+       int len;
+
+       size = strlen(pathname) + sizeof(".new") + 2;
+       if (size > PATH_MAX)
+               return NULL;
+
+       path = malloc(size);
+       if (path == NULL)
+               return NULL;
+
+       len = snprintf(path, size, "%s.new", pathname);
+       if (error_check(len, size)) {
+               free(path);
+               return NULL;
+       }
+
+       return path;
+}
+
+/*
+ * Use "mktemp, write, rename" to update the contents of a file atomically.
+ *
+ * Returns true if completely successful, or false if some error occurred.
+ */
+static _Bool
+nsm_atomic_write(const char *path, const void *buf, const size_t buflen)
+{
+       _Bool result = false;
+       ssize_t len;
+       char *temp;
+       int fd;
+
+       temp = nsm_make_temp_pathname(path);
+       if (temp == NULL) {
+               xlog(L_ERROR, "Failed to create new path for %s", path);
+               goto out;
+       }
+
+       fd = open(temp, O_CREAT | O_TRUNC | O_SYNC | O_WRONLY, 0644);
+       if (fd == -1) {
+               xlog(L_ERROR, "Failed to create %s: %m", temp);
+               goto out;
+       }
+
+       len = write(fd, buf, buflen);
+       if (exact_error_check(len, buflen)) {
+               xlog(L_ERROR, "Failed to write %s: %m", temp);
+               (void)close(fd);
+               (void)unlink(temp);
+               goto out;
+       }
+
+       if (close(fd) == -1) {
+               xlog(L_ERROR, "Failed to close %s: %m", temp);
+               (void)unlink(temp);
+               goto out;
+       }
+
+       if (rename(temp, path) == -1) {
+               xlog(L_ERROR, "Failed to rename %s -> %s: %m",
+                               temp, path);
+               (void)unlink(temp);
+               goto out;
+       }
+
+       /* Ostensibly, a sync(2) is not needed here because
+        * open(O_CREAT), write(O_SYNC), and rename(2) are
+        * already synchronous with persistent storage, for
+        * any file system we care about. */
+
+       result = true;
+
+out:
+       free(temp);
+       return result;
+}
+
 /**
  * nsm_setup_pathnames - set up pathname
  * @progname: C string containing name of program, for error messages
@@ -326,7 +414,6 @@ nsm_get_state(_Bool update)
        int fd, state = 0;
        ssize_t result;
        char *path = NULL;
-       char *newpath = NULL;
 
        path = nsm_make_pathname(NSM_STATE_FILE);
        if (path == NULL) {
@@ -365,54 +452,11 @@ update:
 
        if (update) {
                state += 2;
-
-               newpath = nsm_make_pathname(NSM_STATE_FILE ".new");
-               if (newpath == NULL) {
-                       xlog(L_ERROR,
-                         "Failed to create path for " NSM_STATE_FILE ".new");
-                       state = 0;
-                       goto out;
-               }
-
-               fd = open(newpath, O_CREAT | O_SYNC | O_WRONLY, 0644);
-               if (fd == -1) {
-                       xlog(L_ERROR, "Failed to create %s: %m", newpath);
-                       state = 0;
-                       goto out;
-               }
-
-               result = write(fd, &state, sizeof(state));
-               if (exact_error_check(result, sizeof(state))) {
-                       xlog(L_ERROR, "Failed to write %s: %m", newpath);
-                       (void)close(fd);
-                       (void)unlink(newpath);
-                       state = 0;
-                       goto out;
-               }
-
-               if (close(fd) == -1) {
-                       xlog(L_ERROR, "Failed to close %s: %m", newpath);
-                       (void)unlink(newpath);
-                       state = 0;
-                       goto out;
-               }
-
-               if (rename(newpath, path) == -1) {
-                       xlog(L_ERROR, "Failed to rename %s -> %s: %m",
-                                       newpath, path);
-                       (void)unlink(newpath);
+               if (!nsm_atomic_write(path, &state, sizeof(state)))
                        state = 0;
-                       goto out;
-               }
-
-               /* Ostensibly, a sync(2) is not needed here because
-                * open(O_CREAT), write(O_SYNC), and rename(2) are
-                * already synchronous with persistent storage, for
-                * any file system we care about. */
        }
 
 out:
-       free(newpath);
        free(path);
        return state;
 }