[pacman-dev] [PATCH 1/3] Add missing closedir calls in cache cleanup
Signed-off-by: Dan McGee <dan@archlinux.org> --- src/pacman/sync.c | 4 +++- 1 files changed, 3 insertions(+), 1 deletions(-) diff --git a/src/pacman/sync.c b/src/pacman/sync.c index aa09117..837c2b3 100644 --- a/src/pacman/sync.c +++ b/src/pacman/sync.c @@ -93,11 +93,12 @@ static int sync_cleandb(const char *dbpath, int keep_used) { if(rmrf(path)) { pm_fprintf(stderr, PM_LOG_ERROR, _("could not remove repository directory\n")); + closedir(dir); return(1); } } - } + closedir(dir); return(0); } @@ -215,6 +216,7 @@ static int sync_cleancache(int level) unlink(path); } } + closedir(dir); } else { /* full cleanup */ printf(_("Cache directory: %s\n"), cachedir); -- 1.6.4.4
We didn't look at the return status of sync_cleandb() in sync_cleandb_all(). Make it do so and return it up the call chain. Signed-off-by: Dan McGee <dan@archlinux.org> --- src/pacman/sync.c | 10 ++++++---- 1 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/pacman/sync.c b/src/pacman/sync.c index 837c2b3..e4cd408 100644 --- a/src/pacman/sync.c +++ b/src/pacman/sync.c @@ -103,9 +103,11 @@ static int sync_cleandb(const char *dbpath, int keep_used) { } static int sync_cleandb_all(void) { - const char *dbpath = alpm_option_get_dbpath(); + const char *dbpath; char newdbpath[PATH_MAX]; + int ret = 0; + dbpath = alpm_option_get_dbpath(); printf(_("Database directory: %s\n"), dbpath); if(!yesno(_("Do you want to remove unused repositories?"))) { return(0); @@ -113,13 +115,13 @@ static int sync_cleandb_all(void) { /* The sync dbs were previously put in dbpath/, but are now in dbpath/sync/, * so we will clean everything in dbpath/ (except dbpath/local/ and dbpath/sync/, * and only the unused sync dbs in dbpath/sync/ */ - sync_cleandb(dbpath, 0); + ret += sync_cleandb(dbpath, 0); sprintf(newdbpath, "%s%s", dbpath, "sync/"); - sync_cleandb(newdbpath, 1); + ret += sync_cleandb(newdbpath, 1); printf(_("Database directory cleaned up\n")); - return(0); + return(ret); } static int sync_cleancache(int level) -- 1.6.4.4
Previously we only looked at the first cache directory returned by the library. This allows us to look at all cache directories for cleaning. In addition, change the way we do a full (-Scc) cache cleaning operation. Instead of removing the parent directory, remove each package one-by-one as in the -Sc case. This would be ideal for someone mounting a cache directory over NFS, as it ensures we don't wipe out the mountpoint from underneath the directory. Signed-off-by: Dan McGee <dan@archlinux.org> --- Of the three patches, this is the least tested, aka I've never ran this on a machine of mine yet. Please let me know if you are daring enough and try either an -Sc or -Scc, and whether you had multiple cache directories configured. Thanks! -Dan src/pacman/sync.c | 63 +++++++++++++++++++++++++++-------------------------- 1 files changed, 32 insertions(+), 31 deletions(-) diff --git a/src/pacman/sync.c b/src/pacman/sync.c index e4cd408..a2ef616 100644 --- a/src/pacman/sync.c +++ b/src/pacman/sync.c @@ -126,17 +126,15 @@ static int sync_cleandb_all(void) { static int sync_cleancache(int level) { - /* TODO for now, just mess with the first cache directory */ - alpm_list_t* cachedirs = alpm_option_get_cachedirs(); - const char *cachedir = alpm_list_getdata(cachedirs); + alpm_list_t *i; + alpm_list_t *sync_dbs = alpm_option_get_syncdbs(); + int ret = 0; + + for(i = alpm_option_get_cachedirs(); i; i = alpm_list_next(i)) { + printf(_("Cache directory: %s\n"), (char*)alpm_list_getdata(i)); + } if(level == 1) { - /* incomplete cleanup */ - DIR *dir; - struct dirent *ent; - /* Open up each package and see if it should be deleted, - * depending on the clean method used */ - printf(_("Cache directory: %s\n"), cachedir); switch(config->cleanmethod) { case PM_CLEAN_KEEPINST: if(!yesno(_("Do you want to remove uninstalled packages from cache?"))) { @@ -153,11 +151,23 @@ static int sync_cleancache(int level) return(1); } printf(_("removing old packages from cache...\n")); + } else { + if(!noyes(_("Do you want to remove ALL files from cache?"))) { + return(0); + } + printf(_("removing all files from cache...\n")); + } + + for(i = alpm_option_get_cachedirs(); i; i = alpm_list_next(i)) { + const char *cachedir = alpm_list_getdata(i); + DIR *dir = opendir(cachedir); + struct dirent *ent; - dir = opendir(cachedir); if(dir == NULL) { - pm_fprintf(stderr, PM_LOG_ERROR, _("could not access cache directory\n")); - return(1); + pm_fprintf(stderr, PM_LOG_ERROR, + _("could not access cache directory %s\n"), cachedir); + ret++; + continue; } rewinddir(dir); @@ -166,7 +176,6 @@ static int sync_cleancache(int level) char path[PATH_MAX]; int delete = 1; pmpkg_t *localpkg = NULL, *pkg = NULL; - alpm_list_t *sync_dbs = alpm_option_get_syncdbs(); alpm_list_t *j; if(!strcmp(ent->d_name, ".") || !strcmp(ent->d_name, "..")) { @@ -175,11 +184,20 @@ static int sync_cleancache(int level) /* build the full filepath */ snprintf(path, PATH_MAX, "%s%s", cachedir, ent->d_name); + /* short circuit for removing all packages from cache */ + if(level > 1) { + unlink(path); + continue; + } + /* attempt to load the package, prompt removal on failures as we may have * files here that aren't valid packages. we also don't need a full * load of the package, just the metadata. */ if(alpm_pkg_load(path, 0, &localpkg) != 0 || localpkg == NULL) { if(yesno(_("File %s does not seem to be a valid package, remove it?"), path)) { + if(localpkg) { + alpm_pkg_free(localpkg); + } unlink(path); } continue; @@ -219,26 +237,9 @@ static int sync_cleancache(int level) } } closedir(dir); - } else { - /* full cleanup */ - printf(_("Cache directory: %s\n"), cachedir); - if(!noyes(_("Do you want to remove ALL files from cache?"))) { - return(0); - } - printf(_("removing all files from cache...\n")); - - if(rmrf(cachedir)) { - pm_fprintf(stderr, PM_LOG_ERROR, _("could not remove cache directory\n")); - return(1); - } - - if(makepath(cachedir)) { - pm_fprintf(stderr, PM_LOG_ERROR, _("could not create new cache directory\n")); - return(1); - } } - return(0); + return(ret); } static int sync_synctree(int level, alpm_list_t *syncs) -- 1.6.4.4
On Mon, Sep 21, 2009 at 3:15 PM, Dan McGee <dan@archlinux.org> wrote:
Previously we only looked at the first cache directory returned by the library. This allows us to look at all cache directories for cleaning.
In addition, change the way we do a full (-Scc) cache cleaning operation. Instead of removing the parent directory, remove each package one-by-one as in the -Sc case. This would be ideal for someone mounting a cache directory over NFS, as it ensures we don't wipe out the mountpoint from underneath the directory.
Signed-off-by: Dan McGee <dan@archlinux.org> ---
Of the three patches, this is the least tested, aka I've never ran this on a machine of mine yet. Please let me know if you are daring enough and try either an -Sc or -Scc, and whether you had multiple cache directories configured. Thanks!
Well it looks fine. And I tested it on my box, with one and two cache directories, and a bunch of packages. -Sc and -Scc worked fine. So signoff :)
participants (2)
-
Dan McGee
-
Xavier