Re: [PATCH] Factor out common code for reloading database files

From defc49ccbf2e626155e77ba77a141fa9033473a0 Mon Sep 17 00:00:00 2001
From: Guillem Jover <[email protected]>
Date: Sun, 17 Mar 2024 15:02:45 +0100
Subject: [PATCH] libdpkg: Factor out filesystem database file loading into new
 function

This code is duplicated on several places that load filesystem
databases, refactor it into a new function that takes care of the
(re)loading.

Based-on-patch-by: Simon Richter <[email protected]>
---
 lib/dpkg/Makefile.am        |  1 +
 lib/dpkg/db-fsys-divert.c   | 63 ++++++-------------------
 lib/dpkg/db-fsys-load.c     | 91 +++++++++++++++++++++++++++++++++++++
 lib/dpkg/db-fsys-override.c | 69 +++++++---------------------
 lib/dpkg/db-fsys.h          | 26 +++++++++++
 5 files changed, 149 insertions(+), 101 deletions(-)
 create mode 100644 lib/dpkg/db-fsys-load.c

diff --git a/lib/dpkg/Makefile.am b/lib/dpkg/Makefile.am
index 9ef3a37f7..b0243b157 100644
--- a/lib/dpkg/Makefile.am
+++ b/lib/dpkg/Makefile.am
@@ -76,6 +76,7 @@ libdpkg_la_SOURCES = \
 	db-fsys-digest.c \
 	db-fsys-divert.c \
 	db-fsys-files.c \
+	db-fsys-load.c \
 	db-fsys-override.c \
 	deb-version.c \
 	debug.c \
diff --git a/lib/dpkg/db-fsys-divert.c b/lib/dpkg/db-fsys-divert.c
index e0054bb35..351e8ef87 100644
--- a/lib/dpkg/db-fsys-divert.c
+++ b/lib/dpkg/db-fsys-divert.c
@@ -23,7 +23,6 @@
 #include <compat.h>
 
 #include <sys/types.h>
-#include <sys/stat.h>
 
 #include <errno.h>
 #include <string.h>
@@ -40,78 +39,44 @@
 #include <dpkg/db-fsys.h>
 
 static struct fsys_diversion *diversions = NULL;
-static char *diversionsname;
 
 void
 ensure_diversions(void)
 {
-	static struct stat sb_prev;
-	struct stat sb_next;
+	static struct dpkg_db db = {
+		.name = DIVERSIONSFILE,
+	};
+	enum dpkg_db_error rc;
 	char linebuf[MAXDIVERTFILENAME];
-	static FILE *file_prev;
-	FILE *file;
 	struct fsys_diversion *ov, *oicontest, *oialtname;
 
-	if (diversionsname == NULL)
-		diversionsname = dpkg_db_get_path(DIVERSIONSFILE);
-
-	onerr_abort++;
-
-	file = fopen(diversionsname, "r");
-	if (!file) {
-		if (errno != ENOENT)
-			ohshite(_("failed to open diversions file"));
-	} else {
-		setcloexec(fileno(file), diversionsname);
-
-		if (fstat(fileno(file), &sb_next))
-			ohshite(_("failed to fstat diversions file"));
-
-		/*
-		 * We need to keep the database file open so that the
-		 * filesystem cannot reuse the inode number (f.ex. during
-		 * multiple dpkg-divert invocations in a maintainer script),
-		 * otherwise the following check might turn true, and we
-		 * would skip reloading a modified database.
-		 */
-		if (file_prev &&
-		    sb_prev.st_dev == sb_next.st_dev &&
-		    sb_prev.st_ino == sb_next.st_ino) {
-			fclose(file);
-			onerr_abort--;
-			debug(dbg_general, "%s: same, skipping", __func__);
-			return;
-		}
-		sb_prev = sb_next;
-	}
-	if (file_prev)
-		fclose(file_prev);
-	file_prev = file;
+	rc = dpkg_db_reopen(&db);
+	if (rc == DPKG_DB_SAME)
+		return;
 
 	for (ov = diversions; ov; ov = ov->next) {
 		ov->useinstead->divert->camefrom->divert = NULL;
 		ov->useinstead->divert = NULL;
 	}
 	diversions = NULL;
-	if (!file) {
-		onerr_abort--;
-		debug(dbg_general, "%s: none, resetting", __func__);
+
+	if (rc == DPKG_DB_NONE)
 		return;
-	}
-	debug(dbg_general, "%s: new, (re)loading", __func__);
 
-	while (fgets_checked(linebuf, sizeof(linebuf), file, diversionsname) >= 0) {
+	onerr_abort++;
+
+	while (fgets_checked(linebuf, sizeof(linebuf), db.file, db.pathname) >= 0) {
 		oicontest = nfmalloc(sizeof(*oicontest));
 		oialtname = nfmalloc(sizeof(*oialtname));
 
 		oialtname->camefrom = fsys_hash_find_node(linebuf, FHFF_NONE);
 		oialtname->useinstead = NULL;
 
-		fgets_must(linebuf, sizeof(linebuf), file, diversionsname);
+		fgets_must(linebuf, sizeof(linebuf), db.file, db.pathname);
 		oicontest->useinstead = fsys_hash_find_node(linebuf, FHFF_NONE);
 		oicontest->camefrom = NULL;
 
-		fgets_must(linebuf, sizeof(linebuf), file, diversionsname);
+		fgets_must(linebuf, sizeof(linebuf), db.file, db.pathname);
 		oicontest->pkgset = strcmp(linebuf, ":") ?
 		                    pkg_hash_find_set(linebuf) : NULL;
 		oialtname->pkgset = oicontest->pkgset;
diff --git a/lib/dpkg/db-fsys-load.c b/lib/dpkg/db-fsys-load.c
new file mode 100644
index 000000000..c707f942e
--- /dev/null
+++ b/lib/dpkg/db-fsys-load.c
@@ -0,0 +1,91 @@
+/*
+ * libdpkg - Debian packaging suite library routines
+ * db-fsys-load.c - (re)loading of database of files installed on system
+ *
+ * Copyright © 2008-2024 Guillem Jover <[email protected]>
+ * Copyright © 2022 Simon Richter <[email protected]>
+ *
+ * This is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <https://www.gnu.org/licenses/>.
+ */
+
+#include <config.h>
+#include <compat.h>
+
+#include <sys/types.h>
+#include <sys/stat.h>
+
+#include <errno.h>
+
+#include <dpkg/dpkg.h>
+#include <dpkg/dpkg-db.h>
+#include <dpkg/db-fsys.h>
+#include <dpkg/i18n.h>
+#include <dpkg/debug.h>
+
+enum dpkg_db_error
+dpkg_db_reopen(struct dpkg_db *db)
+{
+	struct stat st_next;
+	FILE *file_next;
+
+	if (db->pathname == NULL)
+		db->pathname = dpkg_db_get_path(db->name);
+
+	onerr_abort++;
+
+	file_next = fopen(db->pathname, "r");
+	if (!file_next) {
+		if (errno != ENOENT)
+			ohshite(_("cannot open %s file"), db->pathname);
+	} else {
+		setcloexec(fileno(file_next), db->pathname);
+
+		if (fstat(fileno(file_next), &st_next))
+			ohshite(_("cannot get %s file metadata"), db->pathname);
+
+		/*
+		 * We need to keep the database file open so that the
+		 * filesystem cannot reuse the inode number (f.ex. during
+		 * multiple dpkg-divert invocations in a maintainer script),
+		 * otherwise the following check might turn true, and we
+		 * would skip reloading a modified database.
+		 */
+		if (db->file &&
+		    db->st.st_dev == st_next.st_dev &&
+		    db->st.st_ino == st_next.st_ino) {
+			fclose(file_next);
+			onerr_abort--;
+
+			debug(dbg_general, "%s: unchanged %s, skipping",
+			      __func__, db->pathname);
+			return DPKG_DB_SAME;
+		}
+		db->st = st_next;
+	}
+	if (db->file)
+		fclose(db->file);
+	db->file = file_next;
+
+	onerr_abort--;
+
+	if (db->file) {
+		debug(dbg_general, "%s: new %s, (re)loading",
+		      __func__, db->pathname);
+		return DPKG_DB_LOAD;
+	} else {
+		debug(dbg_general, "%s: missing %s, resetting",
+		      __func__, db->pathname);
+		return DPKG_DB_NONE;
+	}
+}
diff --git a/lib/dpkg/db-fsys-override.c b/lib/dpkg/db-fsys-override.c
index b74f6cbc2..120bc3dd8 100644
--- a/lib/dpkg/db-fsys-override.c
+++ b/lib/dpkg/db-fsys-override.c
@@ -24,7 +24,6 @@
 #include <compat.h>
 
 #include <sys/types.h>
-#include <sys/stat.h>
 
 #include <errno.h>
 #include <string.h>
@@ -41,8 +40,6 @@
 #include <dpkg/debug.h>
 #include <dpkg/db-fsys.h>
 
-static char *statoverridename;
-
 uid_t
 statdb_parse_uid(const char *str)
 {
@@ -111,76 +108,44 @@ statdb_parse_mode(const char *str)
 void
 ensure_statoverrides(enum statdb_parse_flags flags)
 {
-	static struct stat sb_prev;
-	struct stat sb_next;
-	static FILE *file_prev;
-	FILE *file;
+	static struct dpkg_db db = {
+		.name = STATOVERRIDEFILE,
+	};
+	enum dpkg_db_error rc;
 	char *loaded_list, *loaded_list_end, *thisline, *nextline, *ptr;
 	struct file_stat *fso;
 	struct fsys_namenode *fnn;
 	struct fsys_hash_iter *iter;
 
-	if (statoverridename == NULL)
-		statoverridename = dpkg_db_get_path(STATOVERRIDEFILE);
+	rc = dpkg_db_reopen(&db);
+	if (rc == DPKG_DB_SAME)
+		return;
 
 	onerr_abort++;
 
-	file = fopen(statoverridename, "r");
-	if (!file) {
-		if (errno != ENOENT)
-			ohshite(_("failed to open statoverride file"));
-	} else {
-		setcloexec(fileno(file), statoverridename);
-
-		if (fstat(fileno(file), &sb_next))
-			ohshite(_("failed to fstat statoverride file"));
-
-		/*
-		 * We need to keep the database file open so that the
-		 * filesystem cannot reuse the inode number (f.ex. during
-		 * multiple dpkg-statoverride invocations in a maintainer
-		 * script), otherwise the following check might turn true,
-		 * and we would skip reloading a modified database.
-		 */
-		if (file_prev &&
-		    sb_prev.st_dev == sb_next.st_dev &&
-		    sb_prev.st_ino == sb_next.st_ino) {
-			fclose(file);
-			onerr_abort--;
-			debug(dbg_general, "%s: same, skipping", __func__);
-			return;
-		}
-		sb_prev = sb_next;
-	}
-	if (file_prev)
-		fclose(file_prev);
-	file_prev = file;
-
 	/* Reset statoverride information. */
 	iter = fsys_hash_iter_new();
 	while ((fnn = fsys_hash_iter_next(iter)))
 		fnn->statoverride = NULL;
 	fsys_hash_iter_free(iter);
 
-	if (!file) {
-		onerr_abort--;
-		debug(dbg_general, "%s: none, resetting", __func__);
+	onerr_abort--;
+
+	if (rc == DPKG_DB_NONE)
 		return;
-	}
-	debug(dbg_general, "%s: new, (re)loading", __func__);
 
 	/* If the statoverride list is empty we don't need to bother
 	 * reading it. */
-	if (!sb_next.st_size) {
-		onerr_abort--;
+	if (!db.st.st_size)
 		return;
-	}
 
-	loaded_list = m_malloc(sb_next.st_size);
-	loaded_list_end = loaded_list + sb_next.st_size;
+	onerr_abort++;
+
+	loaded_list = m_malloc(db.st.st_size);
+	loaded_list_end = loaded_list + db.st.st_size;
 
-	if (fd_read(fileno(file), loaded_list, sb_next.st_size) < 0)
-		ohshite(_("reading statoverride file '%.250s'"), statoverridename);
+	if (fd_read(fileno(db.file), loaded_list, db.st.st_size) < 0)
+		ohshite(_("reading statoverride file '%.250s'"), db.pathname);
 
 	thisline = loaded_list;
 	while (thisline < loaded_list_end) {
diff --git a/lib/dpkg/db-fsys.h b/lib/dpkg/db-fsys.h
index a95b29d92..2730dcd34 100644
--- a/lib/dpkg/db-fsys.h
+++ b/lib/dpkg/db-fsys.h
@@ -22,6 +22,10 @@
 #ifndef LIBDPKG_DB_FSYS_H
 #define LIBDPKG_DB_FSYS_H
 
+#include <sys/stat.h>
+
+#include <stdio.h>
+
 #include <dpkg/file.h>
 #include <dpkg/fsys.h>
 
@@ -50,6 +54,28 @@ DPKG_BEGIN_DECLS
 struct pkginfo;
 struct pkgbin;
 
+enum dpkg_db_error {
+	/** The database is new or has changed, should be loaded. */
+	DPKG_DB_LOAD = 0,
+	/** No database file found. */
+	DPKG_DB_NONE = -1,
+	/** The database is already loaded and has not changed. */
+	DPKG_DB_SAME = -2,
+};
+
+struct dpkg_db {
+	/** Name of the database. Set by the caller. */
+	const char *name;
+
+	/* Database state members. */
+	char *pathname;
+	FILE *file;
+	struct stat st;
+};
+
+enum dpkg_db_error
+dpkg_db_reopen(struct dpkg_db *db);
+
 void ensure_diversions(void);
 
 enum statdb_parse_flags {
-- 
2.43.0