[pacman-dev] [PATCH] Make local_db_read() private to the local backend
There is little need to expose the guts of this function even within the library. Make it static in be_local.c, and clean up a few other things since we know exactly where it is being called from: * Remove unnecessary origin checks in _cache_get_*() methods- if you are calling a cache method your package type will be correct. * Remove sanity checks within local_db_read() itself- packages will always have a name and version if they get this far, and the package object will never be NULL either. The one case calling this from outside the backend was in add.c, where we forced a full load of a package before we duplicated it. Move this concern elsewhere and have pkg_dup() always force a full package load via a new force_load() function on the operations callback struct. Signed-off-by: Dan McGee <dan@archlinux.org> --- lib/libalpm/add.c | 3 --- lib/libalpm/be_local.c | 45 +++++++++++++++++---------------------------- lib/libalpm/db.h | 1 - lib/libalpm/package.c | 8 ++++++++ lib/libalpm/package.h | 2 ++ 5 files changed, 27 insertions(+), 32 deletions(-) diff --git a/lib/libalpm/add.c b/lib/libalpm/add.c index 4f6c9d4..3df69f8 100644 --- a/lib/libalpm/add.c +++ b/lib/libalpm/add.c @@ -470,9 +470,6 @@ static int commit_single_pkg(pmhandle_t *handle, pmpkg_t *newpkg, /* we'll need to save some record for backup checks later */ oldpkg = _alpm_pkg_dup(local); - /* make sure all infos are loaded because the database entry - * will be removed soon */ - _alpm_local_db_read(oldpkg->origin_data.db, oldpkg, INFRQ_ALL); EVENT(trans, PM_TRANS_EVT_UPGRADE_START, newpkg, oldpkg); _alpm_log(handle, PM_LOG_DEBUG, "upgrading package %s-%s\n", diff --git a/lib/libalpm/be_local.c b/lib/libalpm/be_local.c index 5006688..108980f 100644 --- a/lib/libalpm/be_local.c +++ b/lib/libalpm/be_local.c @@ -41,10 +41,12 @@ #include "package.h" #include "deps.h" +static int local_db_read(pmpkg_t *info, pmdbinfrq_t inforeq); + #define LAZY_LOAD(info, errret) \ do { \ - if(pkg->origin != PKG_FROM_FILE && !(pkg->infolevel & info)) { \ - _alpm_local_db_read(pkg->origin_data.db, pkg, info); \ + if(!(pkg->infolevel & info)) { \ + local_db_read(pkg, info); \ } \ } while(0) @@ -135,9 +137,7 @@ static alpm_list_t *_cache_get_groups(pmpkg_t *pkg) static int _cache_has_scriptlet(pmpkg_t *pkg) { - if(!(pkg->infolevel & INFRQ_SCRIPTLET)) { - _alpm_local_db_read(pkg->origin_data.db, pkg, INFRQ_SCRIPTLET); - } + LAZY_LOAD(INFRQ_SCRIPTLET, NULL); return pkg->scriptlet; } @@ -179,19 +179,13 @@ static alpm_list_t *_cache_get_deltas(pmpkg_t UNUSED *pkg) static alpm_list_t *_cache_get_files(pmpkg_t *pkg) { - if(pkg->origin == PKG_FROM_LOCALDB - && !(pkg->infolevel & INFRQ_FILES)) { - _alpm_local_db_read(pkg->origin_data.db, pkg, INFRQ_FILES); - } + LAZY_LOAD(INFRQ_FILES, NULL); return pkg->files; } static alpm_list_t *_cache_get_backup(pmpkg_t *pkg) { - if(pkg->origin == PKG_FROM_LOCALDB - && !(pkg->infolevel & INFRQ_FILES)) { - _alpm_local_db_read(pkg->origin_data.db, pkg, INFRQ_FILES); - } + LAZY_LOAD(INFRQ_FILES, NULL); return pkg->backup; } @@ -239,6 +233,11 @@ static int _cache_changelog_close(const pmpkg_t UNUSED *pkg, void *fp) return fclose((FILE *)fp); } +static int _cache_force_load(pmpkg_t *pkg) +{ + return local_db_read(pkg, INFRQ_ALL); +} + /** The local database operations struct. Get package fields through * lazy accessor methods that handle any backend loading and caching @@ -271,6 +270,8 @@ static struct pkg_operations local_pkg_ops = { .changelog_open = _cache_changelog_open, .changelog_read = _cache_changelog_read, .changelog_close = _cache_changelog_close, + + .force_load = _cache_force_load, }; static int checkdbdir(pmdb_t *db) @@ -460,7 +461,7 @@ static int local_db_populate(pmdb_t *db) pkg->handle = db->handle; /* explicitly read with only 'BASE' data, accessors will handle the rest */ - if(_alpm_local_db_read(db, pkg, INFRQ_BASE) == -1) { + if(local_db_read(pkg, INFRQ_BASE) == -1) { _alpm_log(db->handle, PM_LOG_ERROR, _("corrupted database entry '%s'\n"), name); _alpm_pkg_free(pkg); continue; @@ -498,25 +499,13 @@ static char *get_pkgpath(pmdb_t *db, pmpkg_t *info) } -int _alpm_local_db_read(pmdb_t *db, pmpkg_t *info, pmdbinfrq_t inforeq) +static int local_db_read(pmpkg_t *info, pmdbinfrq_t inforeq) { FILE *fp = NULL; char path[PATH_MAX]; char line[1024]; char *pkgpath = NULL; - - if(info == NULL || info->name == NULL || info->version == NULL) { - _alpm_log(db->handle, PM_LOG_DEBUG, - "invalid package entry provided to _alpm_local_db_read, skipping\n"); - return -1; - } - - if(info->origin != PKG_FROM_LOCALDB) { - _alpm_log(db->handle, PM_LOG_DEBUG, - "request to read info for a non-local package '%s', skipping...\n", - info->name); - return -1; - } + pmdb_t *db = info->origin_data.db; /* bitmask logic here: * infolevel: 00001111 diff --git a/lib/libalpm/db.h b/lib/libalpm/db.h index 0318734..b51412a 100644 --- a/lib/libalpm/db.h +++ b/lib/libalpm/db.h @@ -87,7 +87,6 @@ pmdb_t *_alpm_db_register_sync(pmhandle_t *handle, const char *treename, void _alpm_db_unregister(pmdb_t *db); /* be_*.c, backend specific calls */ -int _alpm_local_db_read(pmdb_t *db, pmpkg_t *info, pmdbinfrq_t inforeq); int _alpm_local_db_prepare(pmdb_t *db, pmpkg_t *info); int _alpm_local_db_write(pmdb_t *db, pmpkg_t *info, pmdbinfrq_t inforeq); int _alpm_local_db_remove(pmdb_t *db, pmpkg_t *info); diff --git a/lib/libalpm/package.c b/lib/libalpm/package.c index 75ac94c..f44cd41 100644 --- a/lib/libalpm/package.c +++ b/lib/libalpm/package.c @@ -126,6 +126,8 @@ static int _pkg_changelog_close(const pmpkg_t UNUSED *pkg, return EOF; } +static int _pkg_force_load(pmpkg_t UNUSED *pkg) { return 0; } + /** The standard package operations struct. Get fields directly from the * struct itself with no abstraction layer or any type of lazy loading. */ @@ -157,6 +159,8 @@ struct pkg_operations default_pkg_ops = { .changelog_open = _pkg_changelog_open, .changelog_read = _pkg_changelog_read, .changelog_close = _pkg_changelog_close, + + .force_load = _pkg_force_load, }; /* Public functions for getting package information. These functions @@ -437,6 +441,10 @@ pmpkg_t *_alpm_pkg_dup(pmpkg_t *pkg) pmpkg_t *newpkg; alpm_list_t *i; + if(pkg->ops->force_load(pkg)) { + return NULL; + } + CALLOC(newpkg, 1, sizeof(pmpkg_t), goto cleanup); newpkg->name_hash = pkg->name_hash; diff --git a/lib/libalpm/package.h b/lib/libalpm/package.h index d18020d..bbb4999 100644 --- a/lib/libalpm/package.h +++ b/lib/libalpm/package.h @@ -76,6 +76,8 @@ struct pkg_operations { size_t (*changelog_read) (void *, size_t, const pmpkg_t *, const void *); int (*changelog_close) (const pmpkg_t *, void *); + int (*force_load) (pmpkg_t *); + /* still to add: * checkmd5sum() ? * compute_requiredby() -- 1.7.6
On 29/06/11 13:06, Dan McGee wrote:
There is little need to expose the guts of this function even within the library. Make it static in be_local.c, and clean up a few other things since we know exactly where it is being called from:
* Remove unnecessary origin checks in _cache_get_*() methods- if you are calling a cache method your package type will be correct. * Remove sanity checks within local_db_read() itself- packages will always have a name and version if they get this far, and the package object will never be NULL either.
The one case calling this from outside the backend was in add.c, where we forced a full load of a package before we duplicated it. Move this concern elsewhere and have pkg_dup() always force a full package load via a new force_load() function on the operations callback struct.
Signed-off-by: Dan McGee<dan@archlinux.org>
Seems fine to me. Signed-off-by: Allan
participants (2)
-
Allan McRae
-
Dan McGee