cfg: Write "# This file intentionally left blank.\n" to empty config file.

It's somewhat surprising to see an empty config file--it makes folks wonder
if something went wrong.  If we write a comment, it is more reassuring.

The main part of the change is refactoring.  This may be going a bit
overboard, but it should make the code more obvious and easier to
maintain, I hope.

diff --git a/lib/cfg.c b/lib/cfg.c
index ed177404adc2c2255ba6b66fff2ecd74f4245580..07a7d437257071639c6010145e9d021e5a4366fd 100644 (file)
--- a/lib/cfg.c
+++ b/lib/cfg.c
@@ -292,49 +292,35 @@ cfg_lock(uint8_t *cookie)
     return 0;
 }
 
-/* Write 'new_cfg' into the configuration file.  If 'cookie' is not null, 
- * then the write only occurs if it matches the current configuration's 
- * cookie.  If successful, the old configuration is returned in 'new_cfg'.  
- * Returns 0 if successful, otherwise a negative errno value. */
-int
-cfg_write(void)
+static int
+do_write_config(const void *data, size_t len)
 {
-    size_t i;
-    int fd;
-
-
-    svec_sort(&cfg);
+    FILE *file;
+    int error;
 
-    fd = open(tmp_name, O_CREAT|O_WRONLY|O_TRUNC, S_IRUSR|S_IWUSR);
-    if (fd == -1) {
-        VLOG_WARN("could not open temp config file for writing: %s", 
-                strerror(errno));
+    file = fopen(tmp_name, "w");
+    if (file == NULL) {
+        VLOG_WARN("could not open %s for writing: %s",
+                  tmp_name, strerror(errno));
         return errno;
     }
 
-    for (i = 0; i < cfg.n; i++) {
-        int retval;
-        const char *entry = cfg.names[i];
-        retval = write(fd, entry, strlen(entry));
-        if (retval != strlen(entry)) {
-            VLOG_WARN("problem writing to temp config file %d: %s", 
-                    retval, strerror(errno));
-            close(fd);
-            return errno;
-        }
-        retval = write(fd, "\n", 1);
-        if (retval != 1) {
-            VLOG_WARN("problem writing to temp config file %d: %s", 
-                    retval, strerror(errno));
-            close(fd);
-            return errno;
-        }
+    fwrite(data, 1, len, file);
+
+    /* This is essentially equivalent to:
+     *       error = ferror(file) || fflush(file) || fclose(file);
+     * but it doesn't short-circuit, so that it always closes 'file'. */
+    error = ferror(file);
+    error = fflush(file) || error;
+    error = fclose(file) || error;
+    if (error) {
+        VLOG_WARN("problem writing to %s: %s", tmp_name, strerror(errno));
+        return errno;
     }
-    close(fd);
 
     if (rename(tmp_name, cfg_name) < 0) {
-        VLOG_WARN("could not rename temp config file: %s", 
-                strerror(errno));
+        VLOG_WARN("could not rename %s to %s: %s",
+                  tmp_name, cfg_name, strerror(errno));
         return errno;
     }
 
@@ -343,41 +329,32 @@ cfg_write(void)
     return 0;
 }
 
+/* Write the current configuration into the configuration file.  Returns 0 if
+ * successful, otherwise a negative errno value. */
 int
-cfg_write_data(uint8_t *data, size_t len)
+cfg_write(void)
 {
-    int fd;
+    char *content;
     int retval;
 
-    fd = open(tmp_name, O_CREAT|O_WRONLY|O_TRUNC, S_IRUSR|S_IWUSR);
-    if (fd == -1) {
-        VLOG_WARN("could not open temp config file for writing: %s", 
-                strerror(errno));
-        return errno;
-    }
-
-    while (len > 0) {
-        retval = write(fd, data, len);
-        if (retval < 0) {
-            close(fd);
-            return errno;
-        }
-        len -= retval;
-        data += retval;
-    }
+    svec_sort(&cfg);
+    content = (cfg.n
+               ? svec_join(&cfg, "\n", "\n")
+               : xstrdup("# This file intentionally left blank.\n"));
+    retval = do_write_config(content, strlen(content));
+    free(content);
 
-    close(fd);
+    return retval;
+}
 
-    if (rename(tmp_name, cfg_name) < 0) {
-        VLOG_WARN("could not rename temp config file: %s", 
-                strerror(errno));
-        return errno;
+int
+cfg_write_data(uint8_t *data, size_t len)
+{
+    int retval = do_write_config(data, len);
+    if (!retval) {
+        cfg_read();
     }
-
-    dirty = false;
-    cfg_read();
-
-    return 0;
+    return retval;
 }
 
 void