half-atomically rebuild databases

[Ingo Schwarze <schwarze@usta.de>]

Hi,

when mandocdb(8) rebuilds a database, the old database is already
truncated during the build, so you can't use apropos(1) at that
time, and if mandocdb fails, you have lost your database.

Improve this by first building the database in temporary files,
then rename(2)ing them into place when they are ready.

This is not perfect because the mandocdb process might get -KILLed
between the two renames, but i don't see a syscall anywhere to
atomically rename *two* files.

Also, now it turns out why using mandoc_malloc and friends in
a library is maybe not that smart.  If the library exits,
we have no chance to unlink(2) the temporary files.  Well, we
could rely on atexit(3), but that's exceedingly ugly, and even
the atexit(3) manual itself strongly discourages its use.

All that said, i think the patch below is an improvement.
But it's surprisingly large, so i'd like having somebody
look it over before commit.

Thanks,
  Ingo


Index: mandocdb.c
===================================================================
RCS file: /cvs/src/usr.bin/mandoc/mandocdb.c,v
retrieving revision 1.33
diff -u -p -r1.33 mandocdb.c
--- mandocdb.c	28 Dec 2011 01:17:01 -0000	1.33
+++ mandocdb.c	1 Jan 2012 15:30:54 -0000
@@ -21,6 +21,7 @@
 #include <assert.h>
 #include <ctype.h>
 #include <dirent.h>
+#include <errno.h>
 #include <fcntl.h>
 #include <getopt.h>
 #include <stdio.h>
@@ -382,10 +383,6 @@ mandocdb(int argc, char *argv[])
 	buf.cp = mandoc_malloc(buf.size);
 	dbuf.cp = mandoc_malloc(dbuf.size);
 
-	flags = O_CREAT | O_RDWR;
-	if (OP_DEFAULT == op || OP_CONFFILE == op)
-		flags |= O_TRUNC;
-
 	if (OP_TEST == op) {
 		ofile_argbuild(argc, argv, &of);
 		if (NULL == of)
@@ -408,6 +405,7 @@ mandocdb(int argc, char *argv[])
 			exit((int)MANDOCLEVEL_BADARG);
 		}
 
+		flags = O_CREAT | O_RDWR;
 		mdb.db = dbopen(mdb.dbn, flags, 0644, DB_BTREE, &info);
 		mdb.idx = dbopen(mdb.idxn, flags, 0644, DB_RECNO, NULL);
 
@@ -464,62 +462,89 @@ mandocdb(int argc, char *argv[])
 		manpath_parse(&dirs, dir, NULL, NULL);
 
 	for (i = 0; i < dirs.sz; i++) {
-		mdb.idxn[0] = mdb.dbn[0] = '\0';
-
-		strlcat(mdb.dbn, dirs.paths[i], MAXPATHLEN);
-		strlcat(mdb.dbn, "/", MAXPATHLEN);
-		sz1 = strlcat(mdb.dbn, MANDOC_DB, MAXPATHLEN);
 
-		strlcat(mdb.idxn, dirs.paths[i], MAXPATHLEN);
-		strlcat(mdb.idxn, "/", MAXPATHLEN);
-		sz2 = strlcat(mdb.idxn, MANDOC_IDX, MAXPATHLEN);
+		/*
+		 * Go to the root of the respective manual tree.
+		 * This must work or no manuals may be found:
+		 * They are indexed relative to the root.
+		 */
 
-		if (sz1 >= MAXPATHLEN || sz2 >= MAXPATHLEN) {
-			fprintf(stderr, "%s: path too long\n",
-					dirs.paths[i]);
-			exit((int)MANDOCLEVEL_BADARG);
+		if (-1 == chdir(dirs.paths[i])) {
+			perror(dirs.paths[i]);
+			exit((int)MANDOCLEVEL_SYSERR);
 		}
 
-		if (mdb.db)
-			(*mdb.db->close)(mdb.db);
-		if (mdb.idx)
-			(*mdb.idx->close)(mdb.idx);
+		/* Create a new database in two temporary files. */
 
-		mdb.db = dbopen(mdb.dbn, flags, 0644, DB_BTREE, &info);
-		mdb.idx = dbopen(mdb.idxn, flags, 0644, DB_RECNO, NULL);
-
-		if (NULL == mdb.db) {
-			perror(mdb.dbn);
-			exit((int)MANDOCLEVEL_SYSERR);
-		} else if (NULL == mdb.idx) {
-			perror(mdb.idxn);
-			exit((int)MANDOCLEVEL_SYSERR);
+		flags = O_CREAT | O_EXCL | O_RDWR;
+		while (NULL == mdb.db) {
+			strlcpy(mdb.dbn, MANDOC_DB, MAXPATHLEN);
+			strlcat(mdb.dbn, ".XXXXXXXXXX", MAXPATHLEN);
+			if (NULL == mktemp(mdb.dbn)) {
+				perror(mdb.dbn);
+				exit((int)MANDOCLEVEL_SYSERR);
+			}
+			mdb.db = dbopen(mdb.dbn, flags, 0644,
+					DB_BTREE, &info);
+			if (NULL == mdb.db && EEXIST != errno) {
+				perror(mdb.dbn);
+				exit((int)MANDOCLEVEL_SYSERR);
+			}
 		}
+		while (NULL == mdb.idx) {
+			strlcpy(mdb.idxn, MANDOC_IDX, MAXPATHLEN);
+			strlcat(mdb.idxn, ".XXXXXXXXXX", MAXPATHLEN);
+			if (NULL == mktemp(mdb.idxn)) {
+				perror(mdb.idxn);
+				unlink(mdb.dbn);
+				exit((int)MANDOCLEVEL_SYSERR);
+			}
+			mdb.idx = dbopen(mdb.idxn, flags, 0644,
+					DB_RECNO, NULL);
+			if (NULL == mdb.idx && EEXIST != errno) {
+				perror(mdb.idxn);
+				unlink(mdb.dbn);
+				exit((int)MANDOCLEVEL_SYSERR);
+			}
+		}
+
+		/*
+		 * Search for manuals and fill the new database.
+		 */
 
-		ofile_free(of);
-		of = NULL;
+	       	ofile_dirbuild(".", "", "", 0, &of);
 
-		if (-1 == chdir(dirs.paths[i])) {
-			perror(dirs.paths[i]);
-			exit((int)MANDOCLEVEL_SYSERR);
+		if (NULL != of) {
+			index_merge(of, mp, &dbuf, &buf, hash,
+			     &mdb, &recs);
+			ofile_free(of);
+			of = NULL;
 		}
 
-	       	ofile_dirbuild(".", "", "", 0, &of);
-		if (NULL == of)
-			continue;
+		(*mdb.db->close)(mdb.db);
+		(*mdb.idx->close)(mdb.idx);
+		mdb.db = NULL;
+		mdb.idx = NULL;
 
 		/*
-		 * Go to the root of the respective manual tree.
-		 * This must work or no manuals may be found (they're
-		 * indexed relative to the root).
+		 * Replace the old database with the new one.
+		 * This is not perfectly atomic,
+		 * but i cannot think of a better way.
 		 */
 
-		if (-1 == chdir(dirs.paths[i])) {
-			perror(dirs.paths[i]);
+		if (-1 == rename(mdb.dbn, MANDOC_DB)) {
+			perror(MANDOC_DB);
+			unlink(mdb.dbn);
+			unlink(mdb.idxn);
+			exit((int)MANDOCLEVEL_SYSERR);
+		}
+		if (-1 == rename(mdb.idxn, MANDOC_IDX)) {
+			perror(MANDOC_IDX);
+			unlink(MANDOC_DB);
+			unlink(MANDOC_IDX);
+			unlink(mdb.idxn);
 			exit((int)MANDOCLEVEL_SYSERR);
 		}
-
-		index_merge(of, mp, &dbuf, &buf, hash, &mdb, &recs);
 	}
 
 out:
@@ -737,6 +762,8 @@ index_merge(const struct of *of, struct 
 		}
 		if (ch < 0) {
 			perror("hash");
+			unlink(mdb->dbn);
+			unlink(mdb->idxn);
 			exit((int)MANDOCLEVEL_SYSERR);
 		}
 
--
 To unsubscribe send an email to tech+unsubscribe@mdocml.bsd.lv