* Move several variables into better scope
* const-ify a few variables
* Avoid duplicating filelists if it is unnecessary
* Better handling out out of memory condition when adding file conflicts
  to our list

Signed-off-by: Dan McGee <[email protected]>
---
 lib/libalpm/conflict.c |   84 ++++++++++++++++++++++++++++-------------------
 lib/libalpm/remove.c   |   10 +++---
 2 files changed, 55 insertions(+), 39 deletions(-)

diff --git a/lib/libalpm/conflict.c b/lib/libalpm/conflict.c
index 66441a7..d2734d7 100644
--- a/lib/libalpm/conflict.c
+++ b/lib/libalpm/conflict.c
@@ -284,15 +284,15 @@ static alpm_list_t *add_fileconflict(pmhandle_t *handle,
                const char *name1, const char *name2)
 {
        pmfileconflict_t *conflict;
-       MALLOC(conflict, sizeof(pmfileconflict_t), return NULL);
+       MALLOC(conflict, sizeof(pmfileconflict_t), goto error);
 
        conflict->type = type;
-       STRDUP(conflict->target, name1, return NULL);
-       STRDUP(conflict->file, filestr, return NULL);
+       STRDUP(conflict->target, name1, goto error);
+       STRDUP(conflict->file, filestr, goto error);
        if(name2) {
-               STRDUP(conflict->ctarget, name2, return NULL);
+               STRDUP(conflict->ctarget, name2, goto error);
        } else {
-               STRDUP(conflict->ctarget, "", return NULL);
+               STRDUP(conflict->ctarget, "", goto error);
        }
 
        conflicts = alpm_list_add(conflicts, conflict);
@@ -300,6 +300,9 @@ static alpm_list_t *add_fileconflict(pmhandle_t *handle,
                  filestr, name1, name2 ? name2 : "(filesystem)");
 
        return conflicts;
+
+error:
+       RET_ERR(handle, PM_ERR_MEMORY, conflicts);
 }
 
 void _alpm_fileconflict_free(pmfileconflict_t *conflict)
@@ -375,7 +378,7 @@ alpm_list_t *_alpm_db_find_fileconflicts(pmhandle_t *handle,
         * here as we do when we actually extract files in add.c with our 12
         * different cases. */
        for(current = 0, i = upgrade; i; i = i->next, current++) {
-               alpm_list_t *k, *tmpfiles = NULL;
+               alpm_list_t *k, *tmpfiles;
                pmpkg_t *p1, *p2, *dbpkg;
                char path[PATH_MAX];
 
@@ -391,50 +394,52 @@ alpm_list_t *_alpm_db_find_fileconflicts(pmhandle_t 
*handle,
                _alpm_log(handle, PM_LOG_DEBUG, "searching for file conflicts: 
%s\n",
                                                                
alpm_pkg_get_name(p1));
                for(j = i->next; j; j = j->next) {
+                       alpm_list_t *common_files;
                        p2 = j->data;
                        if(!p2) {
                                continue;
                        }
-                       tmpfiles = filelist_operation( alpm_pkg_get_files(p1),
+                       common_files = 
filelist_operation(alpm_pkg_get_files(p1),
                                        alpm_pkg_get_files(p2), INTERSECT);
 
-                       if(tmpfiles) {
-                               for(k = tmpfiles; k; k = k->next) {
+                       if(common_files) {
+                               for(k = common_files; k; k = k->next) {
                                        snprintf(path, PATH_MAX, "%s%s", 
handle->root, (char *)k->data);
-                                       conflicts = add_fileconflict(handle, 
conflicts, PM_FILECONFLICT_TARGET, path,
+                                       conflicts = add_fileconflict(handle, 
conflicts,
+                                                       PM_FILECONFLICT_TARGET, 
path,
                                                        alpm_pkg_get_name(p1), 
alpm_pkg_get_name(p2));
-                                       if(!conflicts) {
+                                       if(handle->pm_errno == PM_ERR_MEMORY) {
                                                FREELIST(conflicts);
-                                               FREELIST(tmpfiles);
-                                               RET_ERR(handle, PM_ERR_MEMORY, 
NULL);
+                                               FREELIST(common_files);
+                                               return NULL;
                                        }
                                }
-                               FREELIST(tmpfiles);
+                               FREELIST(common_files);
                        }
                }
 
-               /* declarations for second check */
-               struct stat lsbuf, sbuf;
-               char *filestr = NULL;
-
                /* CHECK 2: check every target against the filesystem */
-               _alpm_log(handle, PM_LOG_DEBUG, "searching for filesystem 
conflicts: %s\n", p1->name);
+               _alpm_log(handle, PM_LOG_DEBUG, "searching for filesystem 
conflicts: %s\n",
+                               p1->name);
                dbpkg = _alpm_db_get_pkgfromcache(handle->db_local, p1->name);
 
                /* Do two different checks here. If the package is currently 
installed,
                 * then only check files that are new in the new package. If 
the package
-                * is not currently installed, then simply stat the whole 
filelist */
+                * is not currently installed, then simply stat the whole 
filelist. Note
+                * that the former list needs to be freed while the latter list 
should NOT
+                * be freed. */
                if(dbpkg) {
                        /* older ver of package currently installed */
                        tmpfiles = filelist_operation(alpm_pkg_get_files(p1),
                                        alpm_pkg_get_files(dbpkg), DIFFERENCE);
                } else {
                        /* no version of package currently installed */
-                       tmpfiles = alpm_list_strdup(alpm_pkg_get_files(p1));
+                       tmpfiles = alpm_pkg_get_files(p1);
                }
 
                for(j = tmpfiles; j; j = j->next) {
-                       filestr = j->data;
+                       struct stat lsbuf;
+                       const char *filestr = j->data;
 
                        snprintf(path, PATH_MAX, "%s%s", handle->root, filestr);
 
@@ -442,13 +447,15 @@ alpm_list_t *_alpm_db_find_fileconflicts(pmhandle_t 
*handle,
                        if(_alpm_lstat(path, &lsbuf) != 0) {
                                continue;
                        }
-                       stat(path, &sbuf);
 
                        if(path[strlen(path)-1] == '/') {
+                               struct stat sbuf;
                                if(S_ISDIR(lsbuf.st_mode)) {
                                        _alpm_log(handle, PM_LOG_DEBUG, "%s is 
a directory, not a conflict\n", path);
                                        continue;
-                               } else if(S_ISLNK(lsbuf.st_mode) && 
S_ISDIR(sbuf.st_mode)) {
+                               }
+                               stat(path, &sbuf);
+                               if(S_ISLNK(lsbuf.st_mode) && 
S_ISDIR(sbuf.st_mode)) {
                                        _alpm_log(handle, PM_LOG_DEBUG,
                                                        "%s is a symlink to a 
dir, hopefully not a conflict\n", path);
                                        continue;
@@ -462,7 +469,8 @@ alpm_list_t *_alpm_db_find_fileconflicts(pmhandle_t *handle,
                        for(k = remove; k && !resolved_conflict; k = k->next) {
                                pmpkg_t *rempkg = k->data;
                                if(rempkg && 
alpm_list_find_str(alpm_pkg_get_files(rempkg), filestr)) {
-                                       _alpm_log(handle, PM_LOG_DEBUG, "local 
file will be removed, not a conflict: %s\n", filestr);
+                                       _alpm_log(handle, PM_LOG_DEBUG,
+                                                       "local file will be 
removed, not a conflict: %s\n", filestr);
                                        resolved_conflict = 1;
                                }
                        }
@@ -482,7 +490,8 @@ alpm_list_t *_alpm_db_find_fileconflicts(pmhandle_t *handle,
                                         * by its new owner (whether the file 
is in backup array or not */
                                        handle->trans->skip_remove =
                                                
alpm_list_add(handle->trans->skip_remove, strdup(filestr));
-                                       _alpm_log(handle, PM_LOG_DEBUG, "file 
changed packages, adding to remove skiplist: %s\n", filestr);
+                                       _alpm_log(handle, PM_LOG_DEBUG,
+                                                       "file changed packages, 
adding to remove skiplist: %s\n", filestr);
                                        resolved_conflict = 1;
                                }
                        }
@@ -492,7 +501,8 @@ alpm_list_t *_alpm_db_find_fileconflicts(pmhandle_t *handle,
                                char *dir = malloc(strlen(filestr) + 2);
                                sprintf(dir, "%s/", filestr);
                                
if(alpm_list_find_str(alpm_pkg_get_files(dbpkg),dir)) {
-                                       _alpm_log(handle, PM_LOG_DEBUG, "check 
if all files in %s belongs to %s\n",
+                                       _alpm_log(handle, PM_LOG_DEBUG,
+                                                       "check if all files in 
%s belongs to %s\n",
                                                        dir, dbpkg->name);
                                        resolved_conflict = 
dir_belongsto_pkg(handle->root, filestr, dbpkg);
                                }
@@ -506,23 +516,29 @@ alpm_list_t *_alpm_db_find_fileconflicts(pmhandle_t 
*handle,
                                        continue;
                                }
                                char *filestr = rpath + strlen(handle->root);
-                               
if(alpm_list_find_str(alpm_pkg_get_files(dbpkg),filestr)) {
+                               
if(alpm_list_find_str(alpm_pkg_get_files(dbpkg), filestr)) {
                                        resolved_conflict = 1;
                                }
                                free(rpath);
                        }
 
                        if(!resolved_conflict) {
-                               conflicts = add_fileconflict(handle, conflicts, 
PM_FILECONFLICT_FILESYSTEM,
-                                               path, p1->name, NULL);
-                               if(!conflicts) {
+                               conflicts = add_fileconflict(handle, conflicts,
+                                               PM_FILECONFLICT_FILESYSTEM, 
path, p1->name, NULL);
+                               if(handle->pm_errno == PM_ERR_MEMORY) {
                                        FREELIST(conflicts);
-                                       FREELIST(tmpfiles);
-                                       RET_ERR(handle, PM_ERR_MEMORY, NULL);
+                                       if(dbpkg) {
+                                               /* only freed if it was 
generated from filelist_operation() */
+                                               FREELIST(tmpfiles);
+                                       }
+                                       return NULL;
                                }
                        }
                }
-               FREELIST(tmpfiles);
+               if(dbpkg) {
+                       /* only freed if it was generated from 
filelist_operation() */
+                       FREELIST(tmpfiles);
+               }
        }
        PROGRESS(trans, PM_TRANS_PROGRESS_CONFLICTS_START, "", 100,
                        numtargs, current);
diff --git a/lib/libalpm/remove.c b/lib/libalpm/remove.c
index 264d79e..eada9b9 100644
--- a/lib/libalpm/remove.c
+++ b/lib/libalpm/remove.c
@@ -192,13 +192,13 @@ int _alpm_remove_prepare(pmhandle_t *handle, alpm_list_t 
**data)
 }
 
 static int can_remove_file(pmhandle_t *handle, const char *path,
-               alpm_list_t *skip)
+               alpm_list_t *skip_remove)
 {
        char file[PATH_MAX];
 
        snprintf(file, PATH_MAX, "%s%s", handle->root, path);
 
-       if(alpm_list_find_str(skip, file)) {
+       if(alpm_list_find_str(skip_remove, file)) {
                /* return success because we will never actually remove this 
file */
                return 1;
        }
@@ -209,7 +209,7 @@ static int can_remove_file(pmhandle_t *handle, const char 
*path,
                        /* only return failure if the file ACTUALLY exists and 
we can't write to
                         * it - ignore "chmod -w" simple permission failures */
                        _alpm_log(handle, PM_LOG_ERROR, _("cannot remove file 
'%s': %s\n"),
-                                 file, strerror(errno));
+                                       file, strerror(errno));
                        return 0;
                }
        }
@@ -219,7 +219,7 @@ static int can_remove_file(pmhandle_t *handle, const char 
*path,
 
 /* Helper function for iterating through a package's file and deleting them
  * Used by _alpm_remove_commit. */
-static void unlink_file(pmhandle_t *handle, pmpkg_t *info, char *filename,
+static void unlink_file(pmhandle_t *handle, pmpkg_t *info, const char 
*filename,
                alpm_list_t *skip_remove, int nosave)
 {
        struct stat buf;
@@ -279,7 +279,7 @@ static void unlink_file(pmhandle_t *handle, pmpkg_t *info, 
char *filename,
 
                if(unlink(file) == -1) {
                        _alpm_log(handle, PM_LOG_ERROR, _("cannot remove file 
'%s': %s\n"),
-                                                               filename, 
strerror(errno));
+                                       filename, strerror(errno));
                }
        }
 }
-- 
1.7.5.2


Reply via email to