[pacman-dev] [PATCH 1/3] alpm_unpack : change prefix handling to workaround FS#12148.
Instead of appending the prefix to each entry name, we can chdir to the prefix before extracting, and restoring when it is done. This seems to work better with the strange and special case of FS#12148 where an archive contained the "./" entry. Signed-off-by: Xavier Chantry <shiningxc@gmail.com> --- lib/libalpm/util.c | 31 +++++++++++++++++++++++-------- 1 files changed, 23 insertions(+), 8 deletions(-) diff --git a/lib/libalpm/util.c b/lib/libalpm/util.c index 1bec634..2af2f1f 100644 --- a/lib/libalpm/util.c +++ b/lib/libalpm/util.c @@ -291,16 +291,17 @@ int _alpm_lckrm() */ int _alpm_unpack(const char *archive, const char *prefix, const char *fn) { - int ret = 1; + int ret = 0; mode_t oldmask; struct archive *_archive; struct archive_entry *entry; - char expath[PATH_MAX]; + char cwd[PATH_MAX]; + int restore_cwd = 0; ALPM_LOG_FUNC; if((_archive = archive_read_new()) == NULL) - RET_ERR(PM_ERR_LIBARCHIVE, -1); + RET_ERR(PM_ERR_LIBARCHIVE, 1); archive_read_support_compression_all(_archive); archive_read_support_format_all(_archive); @@ -309,10 +310,25 @@ int _alpm_unpack(const char *archive, const char *prefix, const char *fn) ARCHIVE_DEFAULT_BYTES_PER_BLOCK) != ARCHIVE_OK) { _alpm_log(PM_LOG_ERROR, _("could not open %s: %s\n"), archive, archive_error_string(_archive)); - RET_ERR(PM_ERR_PKG_OPEN, -1); + RET_ERR(PM_ERR_PKG_OPEN, 1); } oldmask = umask(0022); + + /* save the cwd so we can restore it later */ + if(getcwd(cwd, PATH_MAX) == NULL) { + _alpm_log(PM_LOG_ERROR, _("could not get current working directory\n")); + } else { + restore_cwd = 1; + } + + /* just in case our cwd was removed in the upgrade operation */ + if(chdir(prefix) != 0) { + _alpm_log(PM_LOG_ERROR, _("could not change directory to %s (%s)\n"), prefix, strerror(errno)); + ret = 1; + goto cleanup; + } + while(archive_read_next_header(_archive, &entry) == ARCHIVE_OK) { const struct stat *st; const char *entryname; /* the name of the file in the archive */ @@ -337,10 +353,6 @@ int _alpm_unpack(const char *archive, const char *prefix, const char *fn) } /* Extract the archive entry. */ - ret = 0; - snprintf(expath, PATH_MAX, "%s/%s", prefix, entryname); - archive_entry_set_pathname(entry, expath); - int readret = archive_read_extract(_archive, entry, 0); if(readret == ARCHIVE_WARN) { /* operation succeeded but a non-critical error was encountered */ @@ -361,6 +373,9 @@ int _alpm_unpack(const char *archive, const char *prefix, const char *fn) cleanup: umask(oldmask); archive_read_finish(_archive); + if(restore_cwd) { + chdir(cwd); + } return(ret); } -- 1.6.1
These db_open and db_close looked quite useless. And they caused the db directory to be opened on a simple registering of a database. This is totally unneeded, this opening can be delayed to when we actually need it. Signed-off-by: Xavier Chantry <shiningxc@gmail.com> --- lib/libalpm/be_files.c | 40 ++++++++-------------------------------- lib/libalpm/db.c | 15 --------------- 2 files changed, 8 insertions(+), 47 deletions(-) diff --git a/lib/libalpm/be_files.c b/lib/libalpm/be_files.c index de906eb..06b25a9 100644 --- a/lib/libalpm/be_files.c +++ b/lib/libalpm/be_files.c @@ -214,36 +214,6 @@ int SYMEXPORT alpm_db_update(int force, pmdb_t *db) return(0); } -int _alpm_db_open(pmdb_t *db) -{ - ALPM_LOG_FUNC; - - if(db == NULL) { - RET_ERR(PM_ERR_DB_NULL, -1); - } - - _alpm_log(PM_LOG_DEBUG, "opening database from path '%s'\n", db->path); - db->handle = opendir(db->path); - if(db->handle == NULL) { - RET_ERR(PM_ERR_DB_OPEN, -1); - } - - return(0); -} - -void _alpm_db_close(pmdb_t *db) -{ - ALPM_LOG_FUNC; - - if(db == NULL) { - return; - } - - if(db->handle) { - closedir(db->handle); - db->handle = NULL; - } -} static int splitname(const char *target, pmpkg_t *pkg) { @@ -290,13 +260,17 @@ int _alpm_db_populate(pmdb_t *db) struct dirent *ent = NULL; struct stat sbuf; char path[PATH_MAX]; + DIR *dbdir; ALPM_LOG_FUNC; ASSERT(db != NULL, RET_ERR(PM_ERR_DB_NULL, -1)); - rewinddir(db->handle); - while((ent = readdir(db->handle)) != NULL) { + dbdir = opendir(db->path); + if(dbdir == NULL) { + RET_ERR(PM_ERR_DB_OPEN, -1); + } + while((ent = readdir(dbdir)) != NULL) { const char *name = ent->d_name; pmpkg_t *pkg; @@ -311,6 +285,7 @@ int _alpm_db_populate(pmdb_t *db) pkg = _alpm_pkg_new(); if(pkg == NULL) { + closedir(dbdir); return(-1); } /* split the db entry name */ @@ -336,6 +311,7 @@ int _alpm_db_populate(pmdb_t *db) count++; } + closedir(dbdir); db->pkgcache = alpm_list_msort(db->pkgcache, count, _alpm_pkg_cmp); return(count); } diff --git a/lib/libalpm/db.c b/lib/libalpm/db.c index 9b91ce4..a639b1f 100644 --- a/lib/libalpm/db.c +++ b/lib/libalpm/db.c @@ -85,9 +85,6 @@ static void _alpm_db_unregister(pmdb_t *db) return; } - _alpm_log(PM_LOG_DEBUG, "closing database '%s'\n", db->treename); - _alpm_db_close(db); - _alpm_log(PM_LOG_DEBUG, "unregistering database '%s'\n", db->treename); _alpm_db_free(db); } @@ -466,12 +463,6 @@ pmdb_t *_alpm_db_register_local(void) RET_ERR(PM_ERR_DB_CREATE, NULL); } - _alpm_log(PM_LOG_DEBUG, "opening database '%s'\n", db->treename); - if(_alpm_db_open(db) == -1) { - _alpm_db_free(db); - RET_ERR(PM_ERR_DB_OPEN, NULL); - } - handle->db_local = db; return(db); } @@ -522,12 +513,6 @@ pmdb_t *_alpm_db_register_sync(const char *treename) RET_ERR(PM_ERR_DB_CREATE, NULL); } - _alpm_log(PM_LOG_DEBUG, "opening database '%s'\n", db->treename); - if(_alpm_db_open(db) == -1) { - _alpm_db_free(db); - RET_ERR(PM_ERR_DB_OPEN, NULL); - } - handle->dbs_sync = alpm_list_add(handle->dbs_sync, db); return(db); } -- 1.6.1
We don't need to create the directories when local or sync dbs are registered. For example, if a sync db does not exist, we cannot even do "pacman -Q" as an user.. Instead, we can create the local db if needed during the db_prepare operation, and sync dbs on db_update. Also remove some more useless abstractions in db_update and switch to a much more efficient way to remove a sync db : rm -rf. Signed-off-by: Xavier Chantry <shiningxc@gmail.com> --- lib/libalpm/be_files.c | 40 +++++++++++++++++++++++++++++----------- lib/libalpm/db.c | 26 -------------------------- 2 files changed, 29 insertions(+), 37 deletions(-) diff --git a/lib/libalpm/be_files.c b/lib/libalpm/be_files.c index 06b25a9..0faed88 100644 --- a/lib/libalpm/be_files.c +++ b/lib/libalpm/be_files.c @@ -113,6 +113,26 @@ static int setlastupdate(const pmdb_t *db, time_t time) return(ret); } +static int checkdbdir(pmdb_t *db) +{ + struct stat buf; + char *path = db->path; + + if(stat(path, &buf) != 0) { + _alpm_log(PM_LOG_DEBUG, "database dir '%s' does not exist, creating it\n", + path); + if(_alpm_makepath(path) != 0) { + RET_ERR(PM_ERR_SYSTEM, -1); + } + } else if(!S_ISDIR(buf.st_mode)) { + _alpm_log(PM_LOG_WARNING, "removing bogus database: %s\n", path); + if(unlink(path) != 0 || _alpm_makepath(path) != 0) { + RET_ERR(PM_ERR_SYSTEM, -1); + } + } + return(0); +} + /** Update a package database * @param force if true, then forces the update, otherwise update only in case * the database isn't up to date @@ -122,7 +142,6 @@ static int setlastupdate(const pmdb_t *db, time_t time) */ int SYMEXPORT alpm_db_update(int force, pmdb_t *db) { - alpm_list_t *lp; char *dbfile, *dbfilepath; time_t newmtime = 0, lastupdate = 0; const char *dbpath; @@ -176,14 +195,9 @@ int SYMEXPORT alpm_db_update(int force, pmdb_t *db) return(-1); } else { /* remove the old dir */ - _alpm_log(PM_LOG_DEBUG, "flushing database %s\n", db->path); - for(lp = _alpm_db_get_pkgcache(db); lp; lp = lp->next) { - pmpkg_t *pkg = lp->data; - if(pkg && _alpm_db_remove(db, pkg) == -1) { - _alpm_log(PM_LOG_ERROR, _("could not remove database entry %s%s\n"), - db->treename, pkg->name); - RET_ERR(PM_ERR_DB_REMOVE, -1); - } + if(_alpm_rmrf(db->path) != 0) { + _alpm_log(PM_LOG_ERROR, _("could not remove database %s\n"), db->treename); + RET_ERR(PM_ERR_DB_REMOVE, -1); } /* Cache needs to be rebuilt */ @@ -195,6 +209,7 @@ int SYMEXPORT alpm_db_update(int force, pmdb_t *db) sprintf(dbfilepath, "%s%s" DBEXT, dbpath, db->treename); /* uncompress the sync database */ + checkdbdir(db); ret = _alpm_unpack(dbfilepath, db->path, NULL); if(ret) { free(dbfilepath); @@ -268,7 +283,7 @@ int _alpm_db_populate(pmdb_t *db) dbdir = opendir(db->path); if(dbdir == NULL) { - RET_ERR(PM_ERR_DB_OPEN, -1); + return(0); } while((ent = readdir(dbdir)) != NULL) { const char *name = ent->d_name; @@ -634,8 +649,11 @@ int _alpm_db_prepare(pmdb_t *db, pmpkg_t *info) int retval = 0; char *pkgpath = NULL; - oldmask = umask(0000); + if(checkdbdir(db) != 0) { + return(-1); + } + oldmask = umask(0000); pkgpath = get_pkgpath(db, info); if((retval = mkdir(pkgpath, 0755)) != 0) { diff --git a/lib/libalpm/db.c b/lib/libalpm/db.c index a639b1f..561967c 100644 --- a/lib/libalpm/db.c +++ b/lib/libalpm/db.c @@ -428,10 +428,8 @@ alpm_list_t *_alpm_db_search(pmdb_t *db, const alpm_list_t *needles) pmdb_t *_alpm_db_register_local(void) { - struct stat buf; pmdb_t *db; const char *dbpath; - char path[PATH_MAX]; ALPM_LOG_FUNC; @@ -442,21 +440,11 @@ pmdb_t *_alpm_db_register_local(void) _alpm_log(PM_LOG_DEBUG, "registering local database\n"); - /* make sure the database directory exists */ dbpath = alpm_option_get_dbpath(); if(!dbpath) { _alpm_log(PM_LOG_ERROR, _("database path is undefined\n")); RET_ERR(PM_ERR_DB_OPEN, NULL); } - snprintf(path, PATH_MAX, "%slocal", dbpath); - /* TODO this is rediculous, we try to do this even if we can't */ - if(stat(path, &buf) != 0 || !S_ISDIR(buf.st_mode)) { - _alpm_log(PM_LOG_DEBUG, "database dir '%s' does not exist, creating it\n", - path); - if(_alpm_makepath(path) != 0) { - RET_ERR(PM_ERR_SYSTEM, NULL); - } - } db = _alpm_db_new(dbpath, "local"); if(db == NULL) { @@ -469,7 +457,6 @@ pmdb_t *_alpm_db_register_local(void) pmdb_t *_alpm_db_register_sync(const char *treename) { - struct stat buf; pmdb_t *db; const char *dbpath; char path[PATH_MAX]; @@ -487,25 +474,12 @@ pmdb_t *_alpm_db_register_sync(const char *treename) _alpm_log(PM_LOG_DEBUG, "registering sync database '%s'\n", treename); - /* make sure the database directory exists */ dbpath = alpm_option_get_dbpath(); if(!dbpath) { _alpm_log(PM_LOG_ERROR, _("database path is undefined\n")); RET_ERR(PM_ERR_DB_OPEN, NULL); } /* all sync DBs now reside in the sync/ subdir of the dbpath */ - snprintf(path, PATH_MAX, "%ssync/%s", dbpath, treename); - /* TODO this is rediculous, we try to do this even if we can't */ - if(stat(path, &buf) != 0 || !S_ISDIR(buf.st_mode)) { - _alpm_log(PM_LOG_DEBUG, "database dir '%s' does not exist, creating it\n", - path); - if(_alpm_makepath(path) != 0) { - RET_ERR(PM_ERR_SYSTEM, NULL); - } - } - - /* Ensure the db gets the real path. */ - path[0] = '\0'; snprintf(path, PATH_MAX, "%ssync/", dbpath); db = _alpm_db_new(path, treename); -- 1.6.1
These db_open and db_close looked quite useless. And they caused the db directory to be opened on a simple registering of a database. This is totally unneeded, this opening can be delayed to when we actually need it.
Signed-off-by: Xavier Chantry <shiningxc@gmail.com>
After this patch, we don't need db->handle, right?
On Tue, Jan 20, 2009 at 3:03 PM, Nagy Gabor <ngaba@bibl.u-szeged.hu> wrote:
These db_open and db_close looked quite useless. And they caused the db directory to be opened on a simple registering of a database. This is totally unneeded, this opening can be delayed to when we actually need it.
Signed-off-by: Xavier Chantry <shiningxc@gmail.com>
After this patch, we don't need db->handle, right?
He already knows. :) -Dan
participants (3)
-
Dan McGee
-
Nagy Gabor
-
Xavier Chantry