[OpenWrt-Devel] [PATCH] uci: fixes uci leaving junk in /etc/config

[Scott Valentine]

Hello,

The uci package is leaving around a bunch of .*.uci-XXXXX files in /etc/config. The patch below corrects the problem.

Additionally, it cleans up the error checking on the mktemp call (it returns -1 and sets errno on failure).

In any case, the files are all 0 bytes, because there are no relevant config changes, but the mktemp call creates and opens them, causing unnecessary flash writes.

So basically I just deferred the mktemp call so that it will actually get skipped by the "goto done" statement, which avoids the file creation as it should.

Also, do_rename is no longer necessary, as a null check on filename at this point is sufficient, so I eliminated that variable.

Finally, I'm not sure if it is safe, but I reorganized the error handling for the rename call, basically setting ctx->err because the alternative is something like this:

if (filename && rename(filename, p->path)) {
	unlink(filename)
	free(filename)
	THROW(...)
}
if (filename) {
	unlink(filename)
	free(filename)
}

... which kind of makes me cringe a little.

-Scott V.


diff -uNr old/file.c new/file.c
--- old/file.c	2014-07-22 14:32:59.500309320 -1000
+++ new/file.c	2014-07-22 15:26:16.174024969 -1000
@@ -690,7 +690,6 @@
 	char *path = NULL;
 	char *filename = NULL;
 	struct stat statbuf;
-	bool do_rename = false;
 
 	if (!p->path) {
 		if (overwrite)
@@ -699,20 +698,6 @@
 			UCI_THROW(ctx, UCI_ERR_INVAL);
 	}
 
-	if ((asprintf(&filename, "%s/.%s.uci-XXXXXX", ctx->confdir, p->e.name) < 0) || !filename)
-		UCI_THROW(ctx, UCI_ERR_MEM);
-
-	if (!mktemp(filename))
-		*filename = 0;
-
-	if (!*filename) {
-		free(filename);
-		UCI_THROW(ctx, UCI_ERR_IO);
-	}
-
-	if ((stat(filename, &statbuf) == 0) && ((statbuf.st_mode & S_IFMT) != S_IFREG))
-		UCI_THROW(ctx, UCI_ERR_IO);
-
 	/* open the config file for writing now, so that it is locked */
 	f1 = uci_open_stream(ctx, p->path, NULL, SEEK_SET, true, true);
 
@@ -747,6 +732,20 @@
 			goto done;
 	}
 
+	if ((asprintf(&filename, "%s/.%s.uci-XXXXXX", ctx->confdir, p->e.name) < 0) || !filename)
+		UCI_THROW(ctx, UCI_ERR_MEM);
+
+	if (mktemp(filename) < 0) {
+		free(filename);
+		UCI_THROW(ctx, UCI_ERR_IO);
+	}
+
+	if ((stat(filename, &statbuf) == 0) && ((statbuf.st_mode & S_IFMT) != S_IFREG)) {
+		unlink(filename);
+		free(filename);
+		UCI_THROW(ctx, UCI_ERR_IO);
+	}
+
 	f2 = uci_open_stream(ctx, filename, p->path, SEEK_SET, true, true);
 	uci_export(ctx, f2, p, false);
 
@@ -754,19 +753,18 @@
 	fsync(fileno(f2));
 	uci_close_stream(f2);
 
-	do_rename = true;
-
 	UCI_TRAP_RESTORE(ctx);
 
 done:
 	free(name);
 	free(path);
 	uci_close_stream(f1);
-	if (do_rename && rename(filename, p->path)) {
+	if (filename) {
+		if (rename(filename, p->path))
+			ctx->err = UCI_ERR_IO;
 		unlink(filename);
-		UCI_THROW(ctx, UCI_ERR_IO);
+		free(filename);
 	}
-	free(filename);
 	sync();
 	if (ctx->err)
 		UCI_THROW(ctx, ctx->err);
_______________________________________________
openwrt-devel mailing list
openwrt-devel at lists.openwrt.org
https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel