Re: [PATCH] Factor out common code for reloading database files
Hi!
Ah, thanks for the patch! At the time I prepared an alternative, which
I queued in a branch, but then I forgot to mention this on the list,
and to merge it. :/
Attached is what I had, which I've polished a bit more now. Will
include in my next push.
Thanks,
Guillem
From defc49ccbf2e626155e77ba77a141fa9033473a0 Mon Sep 17 00:00:00 2001
From: Guillem Jover <guillem@debian.org>
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 <sjr@debian.org>
---
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 <guillem@debian.org>
+ * Copyright © 2022 Simon Richter <sjr@debian.org>
+ *
+ * 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
Reply to: