This changes the signature of _alpm_pkg_dup() to return an integer error
code and provide the new package in a passed pointer argument. All
callers are now more robust with checking the return value of this
function to ensure a fatal error did not occur.

We allow load failures to proceed as otherwise we have a chicken and egg
problem- if a 'desc' local database entry is missing, the best way of
restoring said file is `pacman -Sf --dbonly packagename`. This patch
fixes a segfault that was occurring in this case.

Fixes the segfault reported in FS#25667.

Signed-off-by: Dan McGee <[email protected]>
---
 lib/libalpm/add.c     |   11 +++++++----
 lib/libalpm/db.c      |    3 +--
 lib/libalpm/deps.c    |   12 +++++++++---
 lib/libalpm/deps.h    |    2 +-
 lib/libalpm/package.c |   28 ++++++++++++++++++++++++----
 lib/libalpm/package.h |    2 +-
 lib/libalpm/remove.c  |   35 ++++++++++++++++++++++++++---------
 lib/libalpm/sync.c    |    6 +++++-
 8 files changed, 74 insertions(+), 25 deletions(-)

diff --git a/lib/libalpm/add.c b/lib/libalpm/add.c
index 8bbf157..cb8551e 100644
--- a/lib/libalpm/add.c
+++ b/lib/libalpm/add.c
@@ -468,19 +468,22 @@ static int commit_single_pkg(alpm_handle_t *handle, 
alpm_pkg_t *newpkg,
                is_upgrade = 1;
 
                /* we'll need to save some record for backup checks later */
-               oldpkg = _alpm_pkg_dup(local);
+               if(_alpm_pkg_dup(local, &oldpkg) == -1) {
+                       ret = -1;
+                       goto cleanup;
+               }
 
-               EVENT(trans, ALPM_TRANS_EVT_UPGRADE_START, newpkg, oldpkg);
+               EVENT(trans, ALPM_TRANS_EVT_UPGRADE_START, newpkg, local);
                _alpm_log(handle, ALPM_LOG_DEBUG, "upgrading package %s-%s\n",
                                newpkg->name, newpkg->version);
 
                /* copy over the install reason */
-               newpkg->reason = alpm_pkg_get_reason(oldpkg);
+               newpkg->reason = alpm_pkg_get_reason(local);
 
                /* pre_upgrade scriptlet */
                if(alpm_pkg_has_scriptlet(newpkg) && !(trans->flags & 
ALPM_TRANS_FLAG_NOSCRIPTLET)) {
                        _alpm_runscriptlet(handle, newpkg->origin_data.file,
-                                       "pre_upgrade", newpkg->version, 
oldpkg->version);
+                                       "pre_upgrade", newpkg->version, 
local->version);
                }
        } else {
                is_upgrade = 0;
diff --git a/lib/libalpm/db.c b/lib/libalpm/db.c
index 77bdf98..33282b8 100644
--- a/lib/libalpm/db.c
+++ b/lib/libalpm/db.c
@@ -553,8 +553,7 @@ int _alpm_db_add_pkgincache(alpm_db_t *db, alpm_pkg_t *pkg)
                return -1;
        }
 
-       newpkg = _alpm_pkg_dup(pkg);
-       if(newpkg == NULL) {
+       if(_alpm_pkg_dup(pkg, &newpkg)) {
                return -1;
        }
 
diff --git a/lib/libalpm/deps.c b/lib/libalpm/deps.c
index 704a904..47b7637 100644
--- a/lib/libalpm/deps.c
+++ b/lib/libalpm/deps.c
@@ -512,13 +512,14 @@ static int can_remove_package(alpm_db_t *db, alpm_pkg_t 
*pkg,
  * @param db package database to do dependency tracing in
  * @param *targs pointer to a list of packages
  * @param include_explicit if 0, explicitly installed packages are not included
+ * @return 0 on success, -1 on errors
  */
-void _alpm_recursedeps(alpm_db_t *db, alpm_list_t *targs, int include_explicit)
+int _alpm_recursedeps(alpm_db_t *db, alpm_list_t *targs, int include_explicit)
 {
        alpm_list_t *i, *j;
 
        if(db == NULL || targs == NULL) {
-               return;
+               return -1;
        }
 
        for(i = targs; i; i = i->next) {
@@ -527,13 +528,18 @@ void _alpm_recursedeps(alpm_db_t *db, alpm_list_t *targs, 
int include_explicit)
                        alpm_pkg_t *deppkg = j->data;
                        if(_alpm_dep_edge(pkg, deppkg)
                                        && can_remove_package(db, deppkg, 
targs, include_explicit)) {
+                               alpm_pkg_t *copy;
                                _alpm_log(db->handle, ALPM_LOG_DEBUG, "adding 
'%s' to the targets\n",
                                                deppkg->name);
                                /* add it to the target list */
-                               targs = alpm_list_add(targs, 
_alpm_pkg_dup(deppkg));
+                               if(_alpm_pkg_dup(deppkg, &copy)) {
+                                       return -1;
+                               }
+                               targs = alpm_list_add(targs, copy);
                        }
                }
        }
+       return 0;
 }
 
 /**
diff --git a/lib/libalpm/deps.h b/lib/libalpm/deps.h
index 29e69eb..ce25bda 100644
--- a/lib/libalpm/deps.h
+++ b/lib/libalpm/deps.h
@@ -31,7 +31,7 @@ void _alpm_dep_free(alpm_depend_t *dep);
 alpm_depend_t *_alpm_dep_dup(const alpm_depend_t *dep);
 void _alpm_depmiss_free(alpm_depmissing_t *miss);
 alpm_list_t *_alpm_sortbydeps(alpm_handle_t *handle, alpm_list_t *targets, int 
reverse);
-void _alpm_recursedeps(alpm_db_t *db, alpm_list_t *targs, int 
include_explicit);
+int _alpm_recursedeps(alpm_db_t *db, alpm_list_t *targs, int include_explicit);
 int _alpm_resolvedeps(alpm_handle_t *handle, alpm_list_t *localpkgs, 
alpm_pkg_t *pkg,
                alpm_list_t *preferred, alpm_list_t **packages, alpm_list_t 
*remove,
                alpm_list_t **data);
diff --git a/lib/libalpm/package.c b/lib/libalpm/package.c
index d865ac9..3d73a43 100644
--- a/lib/libalpm/package.c
+++ b/lib/libalpm/package.c
@@ -467,13 +467,32 @@ alpm_pkg_t *_alpm_pkg_new(void)
        return pkg;
 }
 
-alpm_pkg_t *_alpm_pkg_dup(alpm_pkg_t *pkg)
+/**
+ * Duplicate a package data struct.
+ * @param pkg the package to duplicate
+ * @param new_ptr location to store duplicated package pointer
+ * @return 0 on success, -1 on fatal error, 1 on non-fatal error
+ */
+int _alpm_pkg_dup(alpm_pkg_t *pkg, alpm_pkg_t **new_ptr)
 {
        alpm_pkg_t *newpkg;
        alpm_list_t *i;
+       int ret = 0;
+
+       if(!pkg || !pkg->handle) {
+               return -1;
+       }
+
+       if(!new_ptr) {
+               RET_ERR(pkg->handle, ALPM_ERR_WRONG_ARGS, -1);
+       }
 
        if(pkg->ops->force_load(pkg)) {
-               return NULL;
+               _alpm_log(pkg->handle, ALPM_LOG_WARNING,
+                               _("could not fully load metadata for package 
%s-%s\n"),
+                               pkg->name, pkg->version);
+               ret = 1;
+               pkg->handle->pm_errno = ALPM_ERR_PKG_INVALID;
        }
 
        CALLOC(newpkg, 1, sizeof(alpm_pkg_t), goto cleanup);
@@ -540,11 +559,12 @@ alpm_pkg_t *_alpm_pkg_dup(alpm_pkg_t *pkg)
        newpkg->ops = pkg->ops;
        newpkg->handle = pkg->handle;
 
-       return newpkg;
+       *new_ptr = newpkg;
+       return ret;
 
 cleanup:
        _alpm_pkg_free(newpkg);
-       return NULL;
+       RET_ERR(pkg->handle, ALPM_ERR_MEMORY, -1);
 }
 
 void _alpm_pkg_free(alpm_pkg_t *pkg)
diff --git a/lib/libalpm/package.h b/lib/libalpm/package.h
index bb92ddc..25d1b1a 100644
--- a/lib/libalpm/package.h
+++ b/lib/libalpm/package.h
@@ -144,7 +144,7 @@ alpm_file_t *_alpm_file_copy(alpm_file_t *dest, const 
alpm_file_t *src);
 int _alpm_files_cmp(const void *f1, const void *f2);
 
 alpm_pkg_t* _alpm_pkg_new(void);
-alpm_pkg_t *_alpm_pkg_dup(alpm_pkg_t *pkg);
+int _alpm_pkg_dup(alpm_pkg_t *pkg, alpm_pkg_t **new_ptr);
 void _alpm_pkg_free(alpm_pkg_t *pkg);
 void _alpm_pkg_free_trans(alpm_pkg_t *pkg);
 
diff --git a/lib/libalpm/remove.c b/lib/libalpm/remove.c
index c6886c6..7903a0f 100644
--- a/lib/libalpm/remove.c
+++ b/lib/libalpm/remove.c
@@ -49,6 +49,7 @@ int SYMEXPORT alpm_remove_pkg(alpm_handle_t *handle, 
alpm_pkg_t *pkg)
 {
        const char *pkgname;
        alpm_trans_t *trans;
+       alpm_pkg_t *copy;
 
        /* Sanity checks */
        CHECK_HANDLE(handle, return -1);
@@ -67,11 +68,14 @@ int SYMEXPORT alpm_remove_pkg(alpm_handle_t *handle, 
alpm_pkg_t *pkg)
 
        _alpm_log(handle, ALPM_LOG_DEBUG, "adding package %s to the transaction 
remove list\n",
                        pkgname);
-       trans->remove = alpm_list_add(trans->remove, _alpm_pkg_dup(pkg));
+       if(_alpm_pkg_dup(pkg, &copy) == -1) {
+               return -1;
+       }
+       trans->remove = alpm_list_add(trans->remove, copy);
        return 0;
 }
 
-static void remove_prepare_cascade(alpm_handle_t *handle, alpm_list_t *lp)
+static int remove_prepare_cascade(alpm_handle_t *handle, alpm_list_t *lp)
 {
        alpm_trans_t *trans = handle->trans;
 
@@ -81,14 +85,18 @@ static void remove_prepare_cascade(alpm_handle_t *handle, 
alpm_list_t *lp)
                        alpm_depmissing_t *miss = i->data;
                        alpm_pkg_t *info = 
_alpm_db_get_pkgfromcache(handle->db_local, miss->target);
                        if(info) {
+                               alpm_pkg_t *copy;
                                if(!_alpm_pkg_find(trans->remove, info->name)) {
                                        _alpm_log(handle, ALPM_LOG_DEBUG, 
"pulling %s in target list\n",
                                                        info->name);
-                                       trans->remove = 
alpm_list_add(trans->remove, _alpm_pkg_dup(info));
+                                       if(_alpm_pkg_dup(info, &copy) == -1) {
+                                               return -1;
+                                       }
+                                       trans->remove = 
alpm_list_add(trans->remove, copy);
                                }
                        } else {
-                               _alpm_log(handle, ALPM_LOG_ERROR, _("could not 
find %s in database -- skipping\n"),
-                                                                       
miss->target);
+                               _alpm_log(handle, ALPM_LOG_ERROR,
+                                               _("could not find %s in 
database -- skipping\n"), miss->target);
                        }
                }
                alpm_list_free_inner(lp, (alpm_list_fn_free)_alpm_depmiss_free);
@@ -96,6 +104,7 @@ static void remove_prepare_cascade(alpm_handle_t *handle, 
alpm_list_t *lp)
                lp = alpm_checkdeps(handle, 
_alpm_db_get_pkgcache(handle->db_local),
                                trans->remove, NULL, 1);
        }
+       return 0;
 }
 
 static void remove_prepare_keep_needed(alpm_handle_t *handle, alpm_list_t *lp)
@@ -134,6 +143,7 @@ static void remove_prepare_keep_needed(alpm_handle_t 
*handle, alpm_list_t *lp)
  * the packages blocking the transaction.
  * @param handle the context handle
  * @param data a pointer to an alpm_list_t* to fill
+ * @return 0 on success, -1 on error
  */
 int _alpm_remove_prepare(alpm_handle_t *handle, alpm_list_t **data)
 {
@@ -144,8 +154,10 @@ int _alpm_remove_prepare(alpm_handle_t *handle, 
alpm_list_t **data)
        if((trans->flags & ALPM_TRANS_FLAG_RECURSE)
                        && !(trans->flags & ALPM_TRANS_FLAG_CASCADE)) {
                _alpm_log(handle, ALPM_LOG_DEBUG, "finding removable 
dependencies\n");
-               _alpm_recursedeps(db, trans->remove,
-                               trans->flags & ALPM_TRANS_FLAG_RECURSEALL);
+               if(_alpm_recursedeps(db, trans->remove,
+                               trans->flags & ALPM_TRANS_FLAG_RECURSEALL)) {
+                       return -1;
+               }
        }
 
        if(!(trans->flags & ALPM_TRANS_FLAG_NODEPS)) {
@@ -156,7 +168,9 @@ int _alpm_remove_prepare(alpm_handle_t *handle, alpm_list_t 
**data)
                if(lp != NULL) {
 
                        if(trans->flags & ALPM_TRANS_FLAG_CASCADE) {
-                               remove_prepare_cascade(handle, lp);
+                               if(remove_prepare_cascade(handle, lp)) {
+                                       return -1;
+                               }
                        } else if(trans->flags & ALPM_TRANS_FLAG_UNNEEDED) {
                                /* Remove needed packages (which would break 
dependencies)
                                 * from target list */
@@ -184,7 +198,10 @@ int _alpm_remove_prepare(alpm_handle_t *handle, 
alpm_list_t **data)
        if((trans->flags & ALPM_TRANS_FLAG_CASCADE)
                        && (trans->flags & ALPM_TRANS_FLAG_RECURSE)) {
                _alpm_log(handle, ALPM_LOG_DEBUG, "finding removable 
dependencies\n");
-               _alpm_recursedeps(db, trans->remove, trans->flags & 
ALPM_TRANS_FLAG_RECURSEALL);
+               if(_alpm_recursedeps(db, trans->remove,
+                                       trans->flags & 
ALPM_TRANS_FLAG_RECURSEALL)) {
+                       return -1;
+               }
        }
 
        if(!(trans->flags & ALPM_TRANS_FLAG_NODEPS)) {
diff --git a/lib/libalpm/sync.c b/lib/libalpm/sync.c
index 6c30c94..e3e7fa9 100644
--- a/lib/libalpm/sync.c
+++ b/lib/libalpm/sync.c
@@ -558,8 +558,12 @@ int _alpm_sync_prepare(alpm_handle_t *handle, alpm_list_t 
**data)
                for(j = spkg->removes; j; j = j->next) {
                        alpm_pkg_t *rpkg = j->data;
                        if(!_alpm_pkg_find(trans->remove, rpkg->name)) {
+                               alpm_pkg_t *copy;
                                _alpm_log(handle, ALPM_LOG_DEBUG, "adding '%s' 
to remove list\n", rpkg->name);
-                               trans->remove = alpm_list_add(trans->remove, 
_alpm_pkg_dup(rpkg));
+                               if(_alpm_pkg_dup(rpkg, &copy) == -1) {
+                                       return -1;
+                               }
+                               trans->remove = alpm_list_add(trans->remove, 
copy);
                        }
                }
        }
-- 
1.7.6


Reply via email to