Extract the actual unlinking of files into a new method, which eliminates a goto used for flow control. Also fix up a few small issues in the code: * Unnecessary (unsigned long) cast, use '%zd' instead * Total up errors returned from unlink_file calls and return to caller * Be consistent with scriptlets- we run pre_remove on dbonly, so we should also run post_remove. Both can be disabled by way of the --noscriptlet argument. * Don't pass an invalid pointer to oldpkg to the event callbacks; instead call the callback before we free the object. Signed-off-by: Dan McGee <dan@archlinux.org> --- lib/libalpm/remove.c | 99 +++++++++++++++++++++++++++----------------------- 1 files changed, 54 insertions(+), 45 deletions(-) diff --git a/lib/libalpm/remove.c b/lib/libalpm/remove.c index 8967eff..b43d1a6 100644 --- a/lib/libalpm/remove.c +++ b/lib/libalpm/remove.c @@ -354,38 +354,15 @@ static int unlink_file(alpm_handle_t *handle, alpm_pkg_t *oldpkg, return 0; } -int _alpm_remove_single_package(alpm_handle_t *handle, +static int remove_package_files(alpm_handle_t *handle, alpm_pkg_t *oldpkg, alpm_pkg_t *newpkg, size_t targ_count, size_t pkg_count) { alpm_list_t *skip_remove; - size_t filenum = 0, position = 0; - const char *pkgname = oldpkg->name; - const char *pkgver = oldpkg->version; alpm_filelist_t *filelist; - size_t i; - - if(newpkg) { - _alpm_log(handle, ALPM_LOG_DEBUG, "removing old package first (%s-%s)\n", - pkgname, pkgver); - } else { - EVENT(handle, ALPM_EVENT_REMOVE_START, oldpkg, NULL); - _alpm_log(handle, ALPM_LOG_DEBUG, "removing package %s-%s\n", - pkgname, pkgver); - - /* run the pre-remove scriptlet if it exists */ - if(alpm_pkg_has_scriptlet(oldpkg) && - !(handle->trans->flags & ALPM_TRANS_FLAG_NOSCRIPTLET)) { - char *scriptlet = _alpm_local_db_pkgpath(handle->db_local, - oldpkg, "install"); - _alpm_runscriptlet(handle, scriptlet, "pre_remove", pkgver, NULL, 0); - free(scriptlet); - } - } - - if(handle->trans->flags & ALPM_TRANS_FLAG_DBONLY) { - goto db; - } + size_t i, filenum = 0, position = 0; + int err = 0; + int nosave = handle->trans->flags & ALPM_TRANS_FLAG_NOSAVE; if(newpkg) { alpm_filelist_t *newfiles; @@ -416,32 +393,33 @@ int _alpm_remove_single_package(alpm_handle_t *handle, alpm_file_t *file = filelist->files + i; if(!can_remove_file(handle, file, skip_remove)) { _alpm_log(handle, ALPM_LOG_DEBUG, - "not removing package '%s', can't remove all files\n", pkgname); + "not removing package '%s', can't remove all files\n", + oldpkg->name); + FREELIST(skip_remove); RET_ERR(handle, ALPM_ERR_PKG_CANT_REMOVE, -1); } filenum++; } - _alpm_log(handle, ALPM_LOG_DEBUG, "removing %ld files\n", (unsigned long)filenum); + _alpm_log(handle, ALPM_LOG_DEBUG, "removing %zd files\n", filenum); if(!newpkg) { /* init progress bar, but only on true remove transactions */ - PROGRESS(handle, ALPM_PROGRESS_REMOVE_START, pkgname, 0, + PROGRESS(handle, ALPM_PROGRESS_REMOVE_START, oldpkg->name, 0, pkg_count, targ_count); } /* iterate through the list backwards, unlinking files */ for(i = filelist->count; i > 0; i--) { alpm_file_t *file = filelist->files + i - 1; - int percent; - /* TODO: check return code and handle accordingly */ - unlink_file(handle, oldpkg, newpkg, file, skip_remove, - handle->trans->flags & ALPM_TRANS_FLAG_NOSAVE); + if(unlink_file(handle, oldpkg, newpkg, file, skip_remove, nosave) < 0) { + err++; + } if(!newpkg) { /* update progress bar after each file */ - percent = (position * 100) / filenum; - PROGRESS(handle, ALPM_PROGRESS_REMOVE_START, pkgname, + int percent = (position * 100) / filenum; + PROGRESS(handle, ALPM_PROGRESS_REMOVE_START, oldpkg->name, percent, pkg_count, targ_count); } position++; @@ -450,20 +428,56 @@ int _alpm_remove_single_package(alpm_handle_t *handle, if(!newpkg) { /* set progress to 100% after we finish unlinking files */ - PROGRESS(handle, ALPM_PROGRESS_REMOVE_START, pkgname, 100, + PROGRESS(handle, ALPM_PROGRESS_REMOVE_START, oldpkg->name, 100, pkg_count, targ_count); + } + + return err; +} + +int _alpm_remove_single_package(alpm_handle_t *handle, + alpm_pkg_t *oldpkg, alpm_pkg_t *newpkg, + size_t targ_count, size_t pkg_count) +{ + const char *pkgname = oldpkg->name; + const char *pkgver = oldpkg->version; + + if(newpkg) { + _alpm_log(handle, ALPM_LOG_DEBUG, "removing old package first (%s-%s)\n", + pkgname, pkgver); + } else { + EVENT(handle, ALPM_EVENT_REMOVE_START, oldpkg, NULL); + _alpm_log(handle, ALPM_LOG_DEBUG, "removing package %s-%s\n", + pkgname, pkgver); - /* run the post-remove script if it exists */ + /* run the pre-remove scriptlet if it exists */ if(alpm_pkg_has_scriptlet(oldpkg) && !(handle->trans->flags & ALPM_TRANS_FLAG_NOSCRIPTLET)) { char *scriptlet = _alpm_local_db_pkgpath(handle->db_local, oldpkg, "install"); - _alpm_runscriptlet(handle, scriptlet, "post_remove", pkgver, NULL, 0); + _alpm_runscriptlet(handle, scriptlet, "pre_remove", pkgver, NULL, 0); free(scriptlet); } } -db: + if(!(handle->trans->flags & ALPM_TRANS_FLAG_DBONLY)) { + /* TODO check returned errors if any */ + remove_package_files(handle, oldpkg, newpkg, targ_count, pkg_count); + } + + /* run the post-remove script if it exists */ + if(!newpkg && alpm_pkg_has_scriptlet(oldpkg) && + !(handle->trans->flags & ALPM_TRANS_FLAG_NOSCRIPTLET)) { + char *scriptlet = _alpm_local_db_pkgpath(handle->db_local, + oldpkg, "install"); + _alpm_runscriptlet(handle, scriptlet, "post_remove", pkgver, NULL, 0); + free(scriptlet); + } + + if(!newpkg) { + EVENT(handle, ALPM_EVENT_REMOVE_DONE, oldpkg, NULL); + } + /* remove the package from the database */ _alpm_log(handle, ALPM_LOG_DEBUG, "updating database\n"); _alpm_log(handle, ALPM_LOG_DEBUG, "removing database entry '%s'\n", pkgname); @@ -477,11 +491,6 @@ db: pkgname); } - if(!newpkg) { - /* TODO: awesome! we're passing invalid pointers. */ - EVENT(handle, ALPM_EVENT_REMOVE_DONE, oldpkg, NULL); - } - return 0; } -- 1.7.8.1