[pacman-dev] [PATCH] Ensure fileconflict value is actually a string
When we switched to a file object and not just a simple string, we missed an update along the way here in target-target conflicts. This patch looks large, but it really comes down to one errant (char *) cast before that has been reworked to explicitly point to the alpm_file_t object. The rest is simply code cleanup. Signed-off-by: Dan McGee <dan@archlinux.org> --- Should we take this opportunity to rework the file conflict struct type to be well typed instead of using strings everywhere? struct new_fileconflict_t { alpm_fileconflict_t *type; alpm_file_t *file; (frontend would be responsible for root path prepend) alpm_pkg_t *target; alpm_pkg_t *ctarget; }; could change package1 and package2 in _alpm_conflict_t also to be actual packages as well instead of strings. Thoughts? -Dan lib/libalpm/conflict.c | 30 ++++++++++++++---------------- 1 files changed, 14 insertions(+), 16 deletions(-) diff --git a/lib/libalpm/conflict.c b/lib/libalpm/conflict.c index 1c02f18..14c23f4 100644 --- a/lib/libalpm/conflict.c +++ b/lib/libalpm/conflict.c @@ -276,30 +276,29 @@ static alpm_list_t *filelist_operation(alpm_filelist_t *filesA, return ret; } -/* Adds alpm_fileconflict_t to a conflicts list. Pass the conflicts list, type - * (either ALPM_FILECONFLICT_TARGET or ALPM_FILECONFLICT_FILESYSTEM), a file - * string, and either two package names or one package name and NULL. This is - * a wrapper for former functionality that was done inline. +/* Adds alpm_fileconflict_t to a conflicts list. Pass the conflicts list, the + * conflicting file path, and either two packages or one package and NULL. */ static alpm_list_t *add_fileconflict(alpm_handle_t *handle, - alpm_list_t *conflicts, alpm_fileconflicttype_t type, const char *filestr, - const char *name1, const char *name2) + alpm_list_t *conflicts, const char *filestr, + alpm_pkg_t *pkg1, alpm_pkg_t *pkg2) { alpm_fileconflict_t *conflict; MALLOC(conflict, sizeof(alpm_fileconflict_t), goto error); - conflict->type = type; - STRDUP(conflict->target, name1, goto error); + STRDUP(conflict->target, pkg1->name, goto error); STRDUP(conflict->file, filestr, goto error); - if(name2) { - STRDUP(conflict->ctarget, name2, goto error); + if(pkg2) { + conflict->type = ALPM_FILECONFLICT_TARGET; + STRDUP(conflict->ctarget, pkg2->name, goto error); } else { + conflict->type = ALPM_FILECONFLICT_FILESYSTEM; STRDUP(conflict->ctarget, "", goto error); } conflicts = alpm_list_add(conflicts, conflict); _alpm_log(handle, ALPM_LOG_DEBUG, "found file conflict %s, packages %s and %s\n", - filestr, name1, name2 ? name2 : "(filesystem)"); + filestr, pkg1->name, pkg2 ? pkg2->name : "(filesystem)"); return conflicts; @@ -416,9 +415,9 @@ alpm_list_t *_alpm_db_find_fileconflicts(alpm_handle_t *handle, alpm_list_t *k; char path[PATH_MAX]; for(k = common_files; k; k = k->next) { - snprintf(path, PATH_MAX, "%s%s", handle->root, (char *)k->data); - conflicts = add_fileconflict(handle, conflicts, - ALPM_FILECONFLICT_TARGET, path, p1->name, p2->name); + alpm_file_t *file = k->data; + snprintf(path, PATH_MAX, "%s%s", handle->root, file->name); + conflicts = add_fileconflict(handle, conflicts, path, p1, p2); if(handle->pm_errno == ALPM_ERR_MEMORY) { FREELIST(conflicts); FREELIST(common_files); @@ -567,8 +566,7 @@ alpm_list_t *_alpm_db_find_fileconflicts(alpm_handle_t *handle, } if(!resolved_conflict) { - conflicts = add_fileconflict(handle, conflicts, - ALPM_FILECONFLICT_FILESYSTEM, path, p1->name, NULL); + conflicts = add_fileconflict(handle, conflicts, path, p1, NULL); if(handle->pm_errno == ALPM_ERR_MEMORY) { FREELIST(conflicts); if(dbpkg) { -- 1.7.6.4
On Mon, Sep 26, 2011 at 5:35 PM, Dan McGee <dan@archlinux.org> wrote:
When we switched to a file object and not just a simple string, we missed an update along the way here in target-target conflicts. This patch looks large, but it really comes down to one errant (char *) cast before that has been reworked to explicitly point to the alpm_file_t object. The rest is simply code cleanup.
Signed-off-by: Dan McGee <dan@archlinux.org> ---
Should we take this opportunity to rework the file conflict struct type to be well typed instead of using strings everywhere?
struct new_fileconflict_t { alpm_fileconflict_t *type; alpm_file_t *file; (frontend would be responsible for root path prepend) alpm_pkg_t *target; alpm_pkg_t *ctarget; };
could change package1 and package2 in _alpm_conflict_t also to be actual packages as well instead of strings. Thoughts? On second thought- fileconflict might make sense to use package objects in, but conflict_t looks like it can sometimes be depend-style strings...ugh. We only seem to do that in a few places though...
-Dan
lib/libalpm/conflict.c | 30 ++++++++++++++---------------- 1 files changed, 14 insertions(+), 16 deletions(-)
diff --git a/lib/libalpm/conflict.c b/lib/libalpm/conflict.c index 1c02f18..14c23f4 100644 --- a/lib/libalpm/conflict.c +++ b/lib/libalpm/conflict.c @@ -276,30 +276,29 @@ static alpm_list_t *filelist_operation(alpm_filelist_t *filesA, return ret; }
-/* Adds alpm_fileconflict_t to a conflicts list. Pass the conflicts list, type - * (either ALPM_FILECONFLICT_TARGET or ALPM_FILECONFLICT_FILESYSTEM), a file - * string, and either two package names or one package name and NULL. This is - * a wrapper for former functionality that was done inline. +/* Adds alpm_fileconflict_t to a conflicts list. Pass the conflicts list, the + * conflicting file path, and either two packages or one package and NULL. */ static alpm_list_t *add_fileconflict(alpm_handle_t *handle, - alpm_list_t *conflicts, alpm_fileconflicttype_t type, const char *filestr, - const char *name1, const char *name2) + alpm_list_t *conflicts, const char *filestr, + alpm_pkg_t *pkg1, alpm_pkg_t *pkg2) { alpm_fileconflict_t *conflict; MALLOC(conflict, sizeof(alpm_fileconflict_t), goto error);
- conflict->type = type; - STRDUP(conflict->target, name1, goto error); + STRDUP(conflict->target, pkg1->name, goto error); STRDUP(conflict->file, filestr, goto error); - if(name2) { - STRDUP(conflict->ctarget, name2, goto error); + if(pkg2) { + conflict->type = ALPM_FILECONFLICT_TARGET; + STRDUP(conflict->ctarget, pkg2->name, goto error); } else { + conflict->type = ALPM_FILECONFLICT_FILESYSTEM; STRDUP(conflict->ctarget, "", goto error); }
conflicts = alpm_list_add(conflicts, conflict); _alpm_log(handle, ALPM_LOG_DEBUG, "found file conflict %s, packages %s and %s\n", - filestr, name1, name2 ? name2 : "(filesystem)"); + filestr, pkg1->name, pkg2 ? pkg2->name : "(filesystem)");
return conflicts;
@@ -416,9 +415,9 @@ alpm_list_t *_alpm_db_find_fileconflicts(alpm_handle_t *handle, alpm_list_t *k; char path[PATH_MAX]; for(k = common_files; k; k = k->next) { - snprintf(path, PATH_MAX, "%s%s", handle->root, (char *)k->data); - conflicts = add_fileconflict(handle, conflicts, - ALPM_FILECONFLICT_TARGET, path, p1->name, p2->name); + alpm_file_t *file = k->data; + snprintf(path, PATH_MAX, "%s%s", handle->root, file->name); + conflicts = add_fileconflict(handle, conflicts, path, p1, p2); if(handle->pm_errno == ALPM_ERR_MEMORY) { FREELIST(conflicts); FREELIST(common_files); @@ -567,8 +566,7 @@ alpm_list_t *_alpm_db_find_fileconflicts(alpm_handle_t *handle, }
if(!resolved_conflict) { - conflicts = add_fileconflict(handle, conflicts, - ALPM_FILECONFLICT_FILESYSTEM, path, p1->name, NULL); + conflicts = add_fileconflict(handle, conflicts, path, p1, NULL); if(handle->pm_errno == ALPM_ERR_MEMORY) { FREELIST(conflicts); if(dbpkg) { -- 1.7.6.4
participants (1)
-
Dan McGee