From 02ac41535fee2760d30147abb5914f7e6a100712 Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Thu, 12 Mar 2009 10:00:35 -0700 Subject: [PATCH] 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. --- lib/cfg.c | 103 +++++++++++++++++++++--------------------------------- 1 file changed, 40 insertions(+), 63 deletions(-) diff --git a/lib/cfg.c b/lib/cfg.c index ed177404..07a7d437 100644 --- 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 -- 2.30.2