On Thu, 25 Oct 2012 16:57:37 +0200 Alexander Rødseth <[email protected]> wrote:
> Signed-off-by: Alexander Rødseth <[email protected]> > --- The commit message could be more descriptive of what this actually does. Maybe something like 'report errors from _alpm_remove_single_package'. > lib/libalpm/remove.c | 16 +++++++++++----- > 1 file changed, 11 insertions(+), 5 deletions(-) > > diff --git a/lib/libalpm/remove.c b/lib/libalpm/remove.c > index f189e30..370ec99 100644 > --- a/lib/libalpm/remove.c > +++ b/lib/libalpm/remove.c > @@ -501,7 +501,7 @@ static int remove_package_files(alpm_handle_t *handle, > * @param targ_count current index within the transaction (1-based) > * @param pkg_count the number of packages affected by the transaction > * > - * @return 0 > + * @return 0 on success, -1 if errors occurred > */ > int _alpm_remove_single_package(alpm_handle_t *handle, > alpm_pkg_t *oldpkg, alpm_pkg_t *newpkg, > @@ -509,6 +509,7 @@ int _alpm_remove_single_package(alpm_handle_t *handle, > { > const char *pkgname = oldpkg->name; > const char *pkgver = oldpkg->version; > + int ret = 0; > > if(newpkg) { > _alpm_log(handle, ALPM_LOG_DEBUG, "removing old package first > (%s-%s)\n", > @@ -528,9 +529,13 @@ int _alpm_remove_single_package(alpm_handle_t *handle, > } > } > > + /* remove the package files */ > if(!(handle->trans->flags & ALPM_TRANS_FLAG_DBONLY)) { > - /* TODO check returned errors if any */ > - remove_package_files(handle, oldpkg, newpkg, targ_count, > pkg_count); > + if(remove_package_files(handle, oldpkg, newpkg, targ_count, > pkg_count)) { > + _alpm_log(handle, ALPM_LOG_ERROR, _("could not remove > package files for %s\n"), > + pkgname); > + ret = -1; > + } > } > > /* run the post-remove script if it exists */ > @@ -551,15 +556,16 @@ int _alpm_remove_single_package(alpm_handle_t *handle, > if(_alpm_local_db_remove(handle->db_local, oldpkg) == -1) { > _alpm_log(handle, ALPM_LOG_ERROR, _("could not remove database > entry %s-%s\n"), > pkgname, pkgver); > + ret = -1; > } > + > /* remove the package from the cache */ > if(_alpm_db_remove_pkgfromcache(handle->db_local, oldpkg) == -1) { > _alpm_log(handle, ALPM_LOG_ERROR, _("could not remove entry > '%s' from cache\n"), > pkgname); I think the package cache being in an incorrect state is worth returning an error too. > } > > - /* TODO: useful return values */ > - return 0; > + return ret; > } > > /** This return value is used to avoid running ldconfig when it could break the system. Is it worth differentiating error types so that we can still run ldconfig if possible?
