[pacman-dev] [PATCH] It turns out we do care about directories...

[Allan McRae]

On 06/12/12 22:42, Allan McRae wrote:
> This is a bug that has been around for over a year now.  On a package
> upgrade (either by -S or -U) a new directory could overwrite any file.
> This is caused by the filelist difference calculation ignoring all
> directories and thus no new directories were checked for conflicting
> files on the filesystem.
> 
> Signed-off-by: Allan McRae <allan at archlinux.org>
> ---
> 
> I saw the comment was added to that code in 2011, but I think this issue
> has been around for a very long time before that.

Looks like it was introduced in commit 103dbb9f - 2007-02-13!

> Please look at the patch and give it an ack somewhat urgently.  I intend
> to backport to the Arch Linux pacman package.
> 
>  lib/libalpm/filelist.c | 22 ++++++++--------------
>  1 file changed, 8 insertions(+), 14 deletions(-)
> 
> diff --git a/lib/libalpm/filelist.c b/lib/libalpm/filelist.c
> index bf7645b..f8a5258 100644
> --- a/lib/libalpm/filelist.c
> +++ b/lib/libalpm/filelist.c
> @@ -228,23 +228,17 @@ alpm_list_t *_alpm_filelist_difference(alpm_filelist_t *filesA,
>  		alpm_file_t *fileA = filesA->files + ctrA;
>  		const char *strA = filesA->resolved_path[ctrA];
>  		const char *strB = filesB->resolved_path[ctrB];
> -		/* skip directories, we don't care about them */
> -		if(strA[strlen(strA)-1] == '/') {
> +
> +		int cmp = strcmp(strA, strB);
> +		if(cmp < 0) {
> +			/* item only in filesA, qualifies as a difference */
> +			ret = alpm_list_add(ret, fileA);
>  			ctrA++;
> -		} else if(strB[strlen(strB)-1] == '/') {
> +		} else if(cmp > 0) {
>  			ctrB++;
>  		} else {
> -			int cmp = strcmp(strA, strB);
> -			if(cmp < 0) {
> -				/* item only in filesA, qualifies as a difference */
> -				ret = alpm_list_add(ret, fileA);
> -				ctrA++;
> -			} else if(cmp > 0) {
> -				ctrB++;
> -			} else {
> -				ctrA++;
> -				ctrB++;
> -			}
> +			ctrA++;
> +			ctrB++;
>  		}
>  	}
>  
>