Re: [PATCH 3/6] Conffile database handling functions

[Jonathan Nieder <jrnieder@gmail.com>]

Hi,

For reference when looking at other patches, I made a bunch of
cosmetic changes to make this patch a little easier to read.  This
does not address Guillem’s comments, nor even all of my own.  The
changes have not been considered carefully.  But maybe it can be of
some use nevertheless.

Changes from Sean’s original:

 * clarify commit message
 * use struct pkginfo instead of strings to indicate package
 * add one-line comment summarizing each function’s operation in
    conffiledb.c (to complement the more comprehensive interface
    documentation in the header file)
 * be a little paranoid about null arguments (but I didn’t go all the
    way for some reason: if pkg is non-null, pkg->name is assumed to be
    as well)
 * make sure conffiledb_ensure_db behaves even if conffiledb_path
    returns a path with too few components, by open-coding the
    version using memrchr
 * use fork() and exec() instead of system() to run 'diff'
 * new PROCSAVESTATUS flag for libdpkg subproc library, to support
    fork()/exec()-ing diff.
 * some API documentation clarification
 * various whitespace tweaks
 * mark error messages as candidates for translation
 * escape filename characters with high bit set in more error messages
    (This just helped me figure out path_quote_filename().  It might be
    useful to do this generally if path_quote_filename() leans to
    quote control characters such as newline, but until then I don’t
    think it’s worth adopting.)

I am sending this since I have it around already (to avoid duplicate
effort), but I doubt my changes are suitable lumped together like
this.  Please let me know if any of these changes are actually useful,
and I can write a proper patch.

An interdiff will follow under separate cover.

-- %< --
From: Sean Finney <seanius@debian.org>
Date: Wed, 14 Oct 2009 21:23:06 +0200
Subject: [PATCH] Conffile database handling functions

Currently, each Debian package that wants it (and only some) has
to separately carry out the following tasks:

 * 3-way merging of configuration file changes (updating modified
   conffiles without user intervention)
 * showing the old->new delta for distributed conffiles

To do these itself, dpkg needs a working space to store the old
distributed conffile and manipulate the new one before installing
it.  Add some routines to manipulate these files.

Each stored configuration file has the filename

	<admindir>/conffiles/<base>/<pkg>/<hash>

where

 * <admindir> is the standard dpkg administrative dir
              (i.e., /var/lib/dpkg).
 * <base> is a short string corresponding to which version of the
	  conffiles is stored underneath
          (current, new, resolved, or tmp)
 * <pkg> is a package name
 * <hash> is the md5 hash of the installed location of a
	  package's conffile.  A hash is used to keep the
	  directory structure flat and balanced.

This database is not meant for direct user consumption.  However,
a "dpkg-conffile" command-line interface would allow the user to
perform various forms of inspection with regards to installed
conffiles vs the versions in this database.

Using a configuration file database instead of the current
/var/lib/dpkg/info/*.conffiles lists would also allow for
dynamic registration of configuration files, à la ucf.
---
 lib/dpkg/dpkg.h    |    1 +
 lib/dpkg/subproc.c |   34 ++++----
 lib/dpkg/subproc.h |    1 +
 src/Makefile.am    |    1 +
 src/conffiledb.c   |  247 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 src/conffiledb.h   |  182 ++++++++++++++++++++++++++++++++++++++
 6 files changed, 449 insertions(+), 17 deletions(-)
 create mode 100644 src/conffiledb.c
 create mode 100644 src/conffiledb.h

diff --git a/lib/dpkg/dpkg.h b/lib/dpkg/dpkg.h
index 78a2fe6..abbbd1f 100644
--- a/lib/dpkg/dpkg.h
+++ b/lib/dpkg/dpkg.h
@@ -73,6 +73,7 @@ DPKG_BEGIN_DECLS
 #define STATOVERRIDEFILE  "statoverride"
 #define UPDATESDIR        "updates/"
 #define INFODIR           "info/"
+#define CONFFILEDBDIRROOT "conffiles/" /**< @ref conffilesdb root location */
 #define TRIGGERSDIR       "triggers/"
 #define TRIGGERSFILEFILE  "File"
 #define TRIGGERSDEFERREDFILE "Unincorp"
diff --git a/lib/dpkg/subproc.c b/lib/dpkg/subproc.c
index 5a42bf8..80994e4 100644
--- a/lib/dpkg/subproc.c
+++ b/lib/dpkg/subproc.c
@@ -73,22 +73,24 @@ cu_subproc_signals(int argc, void **argv)
 int
 checksubprocerr(int status, const char *description, int flags)
 {
-	int n;
+	if (flags & PROCWARN)
+		flags |= PROCNOERR;
 
 	if (WIFEXITED(status)) {
-		n = WEXITSTATUS(status);
+		int n = WEXITSTATUS(status);
+
 		if (!n)
 			return 0;
-		if (flags & PROCNOERR)
-			return -1;
 		if (flags & PROCWARN)
 			warning(_("%s returned error exit status %d"),
 			        description, n);
-		else
-			ohshit(_("subprocess %s returned error exit status %d"),
-			       description, n);
+		if (flags & PROCNOERR)
+			return flags & PROCSAVESTATUS ? n : -1;
+		ohshit(_("subprocess %s returned error exit status %d"),
+		       description, n);
 	} else if (WIFSIGNALED(status)) {
-		n = WTERMSIG(status);
+		int n = WTERMSIG(status);
+
 		if (!n)
 			return 0;
 		if ((flags & PROCPIPE) && n == SIGPIPE)
@@ -97,16 +99,14 @@ checksubprocerr(int status, const char *description, int flags)
 			warning(_("%s killed by signal (%s)%s"),
 			        description, strsignal(n),
 			        WCOREDUMP(status) ? _(", core dumped") : "");
-		else
-			ohshit(_("subprocess %s killed by signal (%s)%s"),
-			       description, strsignal(n),
-			       WCOREDUMP(status) ? _(", core dumped") : "");
-	} else {
-		ohshit(_("subprocess %s failed with wait status code %d"),
-		       description, status);
+		if (flags & PROCNOERR)
+			return -1;
+		ohshit(_("subprocess %s killed by signal (%s)%s"),
+		       description, strsignal(n),
+		       WCOREDUMP(status) ? _(", core dumped") : "");
 	}
-
-	return -1;
+	ohshit(_("subprocess %s failed with wait status code %d"),
+	       description, status);
 }
 
 int
diff --git a/lib/dpkg/subproc.h b/lib/dpkg/subproc.h
index d1a0748..6037863 100644
--- a/lib/dpkg/subproc.h
+++ b/lib/dpkg/subproc.h
@@ -34,6 +34,7 @@ void cu_subproc_signals(int argc, void **argv);
 #define PROCPIPE 1
 #define PROCWARN 2
 #define PROCNOERR 4
+#define PROCSAVESTATUS 8
 
 int checksubprocerr(int status, const char *desc, int flags);
 int waitsubproc(pid_t pid, const char *desc, int flags);
diff --git a/src/Makefile.am b/src/Makefile.am
index a29b629..24583de 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -20,6 +20,7 @@ bin_PROGRAMS = \
 dpkg_SOURCES = \
 	archives.c archives.h \
 	cleanup.c \
+	conffiledb.c conffiledb.h \
 	configure.c \
 	depcon.c \
 	enquiry.c \
diff --git a/src/conffiledb.c b/src/conffiledb.c
new file mode 100644
index 0000000..301abce
--- /dev/null
+++ b/src/conffiledb.c
@@ -0,0 +1,247 @@
+#include "conffiledb.h"
+
+#include <config.h>
+
+#include <unistd.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <sys/wait.h>
+#include <fcntl.h>
+#include <stdlib.h>
+#include <errno.h>
+
+#include <dpkg/dpkg.h>
+#include <dpkg/i18n.h>
+#include <dpkg/varbuf.h>
+#include <dpkg/buffer.h>
+#include <dpkg/path.h>
+#include <dpkg/subproc.h>
+#include <dpkg/dpkg-db.h>
+#include "main.h"
+
+const char *conffiledb_base_dirs[] = { "current", "new", "resolved", "tmp" };
+
+/* Find a conffiledb entry, conffiledb, or conffiledb base dir. */
+char *
+conffiledb_path(const struct pkginfo *pkg, const char *path,
+		conffiledb_base base)
+{
+	struct varbuf full_path = VARBUF_INIT;
+	const char *basedir;
+
+	/* sanity check */
+	if (base >= conffiledb_base_COUNT)
+		ohshit(_("conffiledb_path called with unsupported base %x"),
+			base);
+
+	basedir = conffiledb_base_dirs[base];
+	varbufprintf(&full_path, "%s/%s%s/",
+		admindir, CONFFILEDBDIRROOT, basedir);
+
+	if (pkg == NULL)
+		goto done;
+	varbufprintf(&full_path, "%s/", pkg->name);
+
+	if (path == NULL)
+		goto done;
+	varbuf_grow(&full_path, CONFFILEDB_MD5SUM_LEN + 1);
+	buffer_md5(path, full_path.buf + full_path.used,
+		strlen(path));
+	full_path.used += CONFFILEDB_MD5SUM_LEN;
+
+done:
+	debug(dbg_conffdetail, "confffiledb_path(%s, %s, %x) = %s\n",
+		pkg ? pkg->name : "(null)", path ? path : "(null)", base,
+		full_path.buf);
+	return full_path.buf;
+}
+
+/**
+ * Ensure that a particular conffiledb dir exists.
+ *
+ * mkdir -p $CONFFILEDBDIRROOT/$base/$pkg
+ *
+ * @param pkg The package owning the conffiledb dir.
+ * @param base Which conffiledb basedir to use.
+ */
+static void
+conffiledb_ensure_db(const struct pkginfo *pkg, conffiledb_base base)
+{
+	struct stat s;
+	int i;
+	char *dbdir, *end, *separators[3];
+
+	dbdir = conffiledb_path(pkg, NULL, base);
+
+	/* sanity check */
+	if (*dbdir != '/')
+		ohshit(_("conffiledb_path returned a relative path"));
+
+	/*
+	 * Temporarily truncate the string to cheaply traverse up to the
+	 * the parent and its parent. store the locations so we can cheaply
+	 * get back to them later.
+	 */
+	end = dbdir + strlen(dbdir);
+	for (i = 0; i < 3; i++) {
+		while (*end != '/' && end != dbdir)
+			end--;
+		*end = '\0';
+		separators[i] = end;
+	}
+
+	/* Now ensure each directory exists while reconstructing the path. */
+	for (i = 2; i >= 0; i--) {
+		if (stat(dbdir, &s)) {
+			debug(dbg_conffdetail,
+			      "conffiledb_ensure_db: creating %s\n", dbdir);
+			if (errno != ENOENT)
+				ohshite("conffiledb_ensure_db: stat(%s)",
+				        dbdir);
+			if (mkdir(dbdir, S_IRWXU))
+				ohshite("conffiledb_ensure_db: mkdir(%s)",
+				        dbdir);
+		}
+		*separators[i] = '/';
+	}
+
+	free(dbdir);
+}
+
+/* Add a new entry to $CONFFILEDBDIRROOT/new/$pkg. */
+void
+conffiledb_register_new_conffile(const struct pkginfo *pkg,
+		const char *path, int fd, size_t sz)
+{
+	int cfgfd;
+	char *dest;
+	char msg_dest[256];
+
+	/* sanity check */
+	if (path == NULL || pkg == NULL)
+		ohshit(_("conffiledb_register_new_conffile called "
+			"with NULL arguments"));
+
+	dest = conffiledb_path(pkg, path, conffiledb_base_new);
+	path_quote_filename(msg_dest, dest, sizeof(msg_dest));
+	debug(dbg_conff, "conffile_register_new_conffile: %s, %s, %s\n",
+		pkg->name, path, msg_dest);
+
+	conffiledb_ensure_db(pkg, conffiledb_base_new);
+
+	cfgfd = open(dest, O_CREAT|O_EXCL|O_WRONLY, S_IRUSR|S_IWUSR);
+	if (cfgfd < 0)
+		ohshite(_("unable to create `%.255s'"), msg_dest);
+	fd_fd_copy(fd, cfgfd, sz, _("new conffiledb entry '%.255s'"),
+	           msg_dest);
+
+	if (close(cfgfd))
+		ohshite("can't close %.255s", msg_dest);
+	free(dest);
+}
+
+/* Open a conffiledb entry for reading. */
+int
+conffiledb_get_conffile(const struct pkginfo *pkg,
+		const char *path, conffiledb_base base)
+{
+	int fd;
+	char *p = conffiledb_path(pkg, path, base);
+
+	fd = open(p, O_RDONLY);
+	if (fd == -1) {
+		char msg_p[256];
+		ohshite(_("error opening conffile registered at %.255s"),
+			path_quote_filename(msg_p, p, sizeof(msg_p)));
+	}
+
+	free(p);
+	return fd;
+}
+
+/*
+ * Update the conffiles "db" dir.  This consists of the following steps:
+ * - nuke the tmp dir if it exists
+ * - rename to_base to the tmp dir
+ * - rename from_base to to_base
+ * - nuke the tmp dir
+ */
+void
+conffiledb_commit(const struct pkginfo *pkg, conffiledb_base from_base,
+                  conffiledb_base to_base)
+{
+	char *cfdb_to = NULL, *cfdb_from = NULL, *cfdb_tmp = NULL;
+
+	/* sanity check */
+	if (pkg == NULL)
+		ohshit(_("conffiledb_commit called for NULL package"));
+
+	debug(dbg_conff, "conffiledb_commit(%s, %d, %d)",
+		pkg->name, from_base, to_base);
+
+	cfdb_from = conffiledb_path(pkg, NULL, from_base);
+	cfdb_to = conffiledb_path(pkg, NULL, to_base);
+	cfdb_tmp = conffiledb_path(pkg, NULL, conffiledb_base_tmp);
+
+	conffiledb_ensure_db(pkg, conffiledb_base_tmp);
+	conffiledb_remove(pkg, conffiledb_base_tmp);
+
+	conffiledb_ensure_db(pkg, to_base);
+	if (rename(cfdb_to, cfdb_tmp))
+		ohshite(_("conffiledb_commit: rename(%s, %s)"),
+			cfdb_to, cfdb_tmp);
+	conffiledb_ensure_db(pkg, from_base);
+	if (rename(cfdb_from, cfdb_to))
+		ohshite(_("conffiledb_commit: rename(%s, %s)"),
+			cfdb_from, cfdb_to);
+
+	conffiledb_remove(pkg, conffiledb_base_tmp);
+
+	free(cfdb_from);
+	free(cfdb_to);
+	free(cfdb_tmp);
+}
+
+/* Remove a conffiledb with all of its entries. */
+void
+conffiledb_remove(const struct pkginfo *pkg, conffiledb_base base)
+{
+	char *cfdb;
+
+	/* sanity check */
+	if (pkg == NULL)
+		ohshit(_("conffiledb_remove called for NULL package"));
+
+	cfdb = conffiledb_path(pkg, NULL, base);
+	debug(dbg_conff, "conffiledb_remove(%s): removing %s\n",
+		pkg->name, cfdb);
+	ensure_pathname_nonexisting(cfdb);
+
+	free(cfdb);
+}
+
+/* Spawn 'diff -u <conffiledb entry> <installed conffile>'. */
+int
+conffiledb_diff(const struct pkginfo *pkg, const char *path,
+		conffiledb_base base)
+{
+	int child, result;
+	char *src;
+
+	/* sanity check */
+	if (pkg == NULL || path == NULL)
+		ohshit(_("conffiledb_diff called for NULL conffile"));
+
+	src = conffiledb_path(pkg, path, base);
+
+	if (!(child = m_fork())) {
+		execlp(DIFF, "diff", "-u", src, path, NULL);
+		ohshite(_("failed to exec 'diff -u %s %s'"),
+			src, path);
+	}
+
+	result = waitsubproc(child, "diff", PROCNOERR | PROCSAVESTATUS);
+
+	free(src);
+	return result;
+}
diff --git a/src/conffiledb.h b/src/conffiledb.h
new file mode 100644
index 0000000..5c8185c
--- /dev/null
+++ b/src/conffiledb.h
@@ -0,0 +1,182 @@
+/**
+ * @file conffiledb.h
+ *
+ * Functions for tracking referenceable versions of packages' conffiles.
+ *
+ * This library allows for tracking the pristine versions (and in some specific
+ * cases a few other versions) of packages' conffiles.  This allows for a
+ * number of desirable features, such as conffile merging and more
+ * featureful reporting of the differences between conffiles on package
+ * upgrade even when the local admin may have modified the installed
+ * version of the file.
+ *
+ * The internal database of conffiles follows the convention:
+ *
+ *	#CONFFILEDBDIRROOT/\<base\>/\<pkg\>/\<md5\>
+ *
+ * where:
+ * - @b \<base\> is a short string describing which version of the conffile
+ *   this is. See enum #conffiledb_base for the enumerated values and
+ *   #conffiledb_base_dirs for the corresponding strings.
+ * - @b \<pkg\> is the package name.
+ * - @b \<md5\> is the md5 hash of the location of a package's conffile.  A
+ *   hash is used to keep a flat and balanced directory structure, and has
+ *   the added benefit of a simpler implementation.
+ *
+ * Some further terminology to keep things clear later on:
+ *
+ * - @b "conffiledb file" or @b "conffiledb entry" refers to a file whose
+ *   contents correspond to a particular conffiledb_base/package/conffile
+ *   triplet.
+ * - @anchor conffiledb @b "conffile database" or @b "conffiledb" refers
+ *   to a directory specific to a conffiledb_base + package tuple.  Such a
+ *   directory will contain copies of the conffiles named after the hash
+ *   of their respective installed locations.
+ * - @b "conffiledb basedir" refers to the subdirectory directly
+ *   underneath the conffiledb root directory, which divides the the
+ *   different conffiledbs by different "reference types" (i.e. the
+ *   original conffile provided by the currently installed package vs.
+ *   a newly unpacked version). The available basedirs are defined in
+ *   conffiledb_base_dirs, which is indexed on the respective enumerated
+ *   values from conffiledb_base.
+ * - @b "conffiledb root" refers to the top-most directory containing
+ *   all conffile databases, and is defined by #CONFFILEDBDIRROOT.
+ */
+#ifndef CONFFILES_H
+#define CONFFILES_H
+
+#include <sys/types.h>
+
+/* Defined in dpkg/dpkg-db.h. */
+struct pkginfo;
+
+/** The different base reference types underneath the conffile db. */
+typedef enum {
+	conffiledb_base_current, /**< The currently installed version */
+	conffiledb_base_new, /**< A newly unpacked version */
+	conffiledb_base_resolved, /**< The most recently resolved conflict */
+	conffiledb_base_tmp, /**< A temporary version for internal use */
+	conffiledb_base_COUNT /**< The count of possible values for this type */
+} conffiledb_base;
+
+/**
+ * The directory names of the conffiledb base dirs, indexed by the respective
+ * conffiledb_base value.
+ */
+extern const char *conffiledb_base_dirs[conffiledb_base_COUNT];
+
+/** Length of the MD5 string used in conffile path hashing (NULL not counted) */
+#define CONFFILEDB_MD5SUM_LEN 32
+
+/**
+ * Determine the canonical path for a conffiledb entry.
+ *
+ * This is a helper function to resolve the full path to a specific location
+ * within the conffile database. The primary use is to find the location
+ * of a conffiledb file, but it can also be used to find the location
+ * of the @ref conffiledb for a particular conffiledb_base + package,
+ * as well as the parent conffiledb basedir corresponding to a particular
+ * conffiledb_base.
+ *
+ * @param pkg The package name. If NULL, the returned value will be
+ *        the conffiledb basedir corresponding to base.
+ * @param path The absolute path to the conffile. If NULL, the returned
+ *        value will be the conffiledb directory which should contain all
+ *        conffiledb files for the combination of pkg and base.
+ * @param base Which conffiledb base reference type is requested.
+ *
+ * @return An allocated string with the path (your job to free it).
+ */
+char *conffiledb_path(const struct pkginfo *pkg, const char *path,
+	conffiledb_base base);
+
+/**
+ * Add a new conffiledb entry for a package.
+ *
+ * For use when unpacking a package.  Create a conffiledb entry in the
+ * conffiledb dir for the #conffiledb_base_new version of a particular
+ * package. The conffile will be later processed in the package configuration
+ * stage, after which there should be a call to conffiledb_commit() before
+ * configuration can be considered successful.
+ *
+ * The use of a file descriptor allows the new entry to be written very early
+ * in the unpack phase, before it exists on disk.  This interface is designed
+ * to hook cleanly with what's available, which is filedescriptor representing
+ * output from tar extraction.
+ *
+ * @param pkg The package owning the conffile.
+ * @param path The installed location of the conffile.
+ * @param fd A file descriptor holding the contents of the conffile.
+ * @param sz Size of the conffile in bytes.
+ *           (XXX why is this not off_t, -1 allowed?)
+ */
+void conffiledb_register_new_conffile(const struct pkginfo *pkg,
+	const char *path, int fd, size_t sz);
+
+/**
+ * Open a conffiledb entry for reading.
+ *
+ * @param pkg The package owning the conffile.
+ * @param path The installed location of the conffile.
+ * @param base Which conffiledb basedir to use.
+ *
+ * @return A read-only file descriptor for the registered conffile
+ *         (your job to close it).
+ */
+int conffiledb_get_conffile(const struct pkginfo *pkg,
+	const char *path, conffiledb_base base);
+
+/**
+ * Remove a package’s conffiledb.
+ *
+ * Remove the indicated version of the specified package’s conffiledb.
+ *
+ * This should be useful in two cases:
+ * - purging a package’s configuration files
+ * - replacing one conffiledb with another.
+ *
+ * @param pkg The package owning the conffile db to be removed.
+ * @param base Which conffiledb basedir to use with respect to pkg.
+ */
+void conffiledb_remove(const struct pkginfo *pkg, conffiledb_base base);
+
+/**
+ * Move a conffiledb from one base reference type to another.
+ *
+ * This is a safer way to perform the operation
+ *  
+ *	rm -r $CONFFILEDBDIRROOT/$to_base/$pkg
+ *	mv $CONFFILEDBDIRROOT/$from_base/$pkg $CONFFILEDBDIRROOT/$to_base/
+ *
+ * The conffiledb corresponding to pkg and the tmp base reference type
+ * is used as a temporary location to hold the entries from to_base until
+ * the entries from from_base are put into place.
+ *
+ * Any entries already in the tmp/$pkg conffiledb will be removed.  The
+ * invoking program is responsible for putting things back in order upon
+ * failure.
+ *
+ * @param pkg The package owning the conffile database
+ * @param from_base The current conffiledb basedir which should be renamed.
+ * @param to_base The conffiledb basedir which should be replaced by the
+ *        version in from_base.
+ */
+void conffiledb_commit(const struct pkginfo *pkg,
+	conffiledb_base from_base, conffiledb_base to_base);
+
+/**
+ * Print a diff between a conffiledb entry and the installed version.
+ *
+ * Print a unified diff between a conffiledb entry and the corresponding
+ * installed conffile to stdout.
+ *
+ * @param pkg The package that owns the conffile.
+ * @param path The pathname to the installed version of the conffile.
+ * @param base Which conffiledb basedir to use.
+ *
+ * @return The exit status from diff(1).
+ */
+int conffiledb_diff(const struct pkginfo *pkg, const char *path,
+	conffiledb_base base);
+
+#endif /* CONFFILES_H */
-- 
1.6.5.2