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 <guil...@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 <s...@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 <guil...@debian.org> + * Copyright © 2022 Simon Richter <s...@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