From: Chuck Lever <chuck.lever@oracle.com>
Date: Thu, 14 Jan 2010 17:23:53 +0000 (-0500)
Subject: libnsm.a: Factor atomic write code out of nsm_get_state()
X-Git-Tag: nfs-utils-1-2-2-rc7~9
X-Git-Url: https://git.decadent.org.uk/gitweb/?p=nfs-utils.git;a=commitdiff_plain;h=b148d3414a8d574ff7883ad99d3d1dd980a12603

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 fc3241a..10769d9 100644
--- 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;
 }