[pacman-dev] [PATCH] Be more robust when copying package data
Dan McGee
dan at archlinux.org
Fri Aug 19 12:11:00 EDT 2011
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 <dan at archlinux.org>
---
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, ©)) {
+ 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, ©) == -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, ©) == -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, ©) == -1) {
+ return -1;
+ }
+ trans->remove = alpm_list_add(trans->remove, copy);
}
}
}
--
1.7.6
More information about the pacman-dev
mailing list