[pacman-dev] [PATCH] Fix Bug FS#31558 and All Other Instances of the Problem

[Allan McRae]

On 22/11/12 10:38, Richard Pougnet wrote:
> Provide full path names in warning messages by printing filename instead
> of entryname.
> 
> Signed-off-by: Richard Pougnet <richard at pougnet.ca>
> ---

Thanks for the patch.  I wonder if this goes too far in places (see
below).   Anyone else got comments?

>  lib/libalpm/add.c | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/lib/libalpm/add.c b/lib/libalpm/add.c
> index edddc31..77fc81f 100644
> --- a/lib/libalpm/add.c
> +++ b/lib/libalpm/add.c
> @@ -178,9 +178,9 @@ static int extract_single_file(alpm_handle_t *handle, struct archive *archive,
>  	/* if a file is in NoExtract then we never extract it */
>  	if(alpm_list_find(handle->noextract, entryname, _alpm_fnmatch)) {
>  		_alpm_log(handle, ALPM_LOG_DEBUG, "%s is in NoExtract, skipping extraction\n",
> -				entryname);
> +				filename);

Here the value in NoExtract is "entryname".  So, I think it makes more
sense to keep it.  Perhaps:

_alpm_log(handle, ALPM_LOG_DEBUG, "%s is in NoExtract, skipping
extraction of %s\n", entryname, filename).

That could be redundant, but that is fine in debug output...

>  		alpm_logaction(handle, "note: %s is in NoExtract, skipping extraction\n",
> -				entryname);
> +				filename);

Here I would just stick to entryname.

The rest seem fine to me.

>  		archive_read_data_skip(archive);
>  		return 0;
>  	}
> @@ -217,20 +217,20 @@ static int extract_single_file(alpm_handle_t *handle, struct archive *archive,
>  					/* if filesystem perms are different than pkg perms, warn user */
>  					mode_t mask = 07777;
>  					_alpm_log(handle, ALPM_LOG_WARNING, _("directory permissions differ on %s\n"
> -								"filesystem: %o  package: %o\n"), entryname, lsbuf.st_mode & mask,
> +								"filesystem: %o  package: %o\n"), filename, lsbuf.st_mode & mask,
>  							entrymode & mask);
>  					alpm_logaction(handle, "warning: directory permissions differ on %s\n"
> -							"filesystem: %o  package: %o\n", entryname, lsbuf.st_mode & mask,
> +							"filesystem: %o  package: %o\n", filename, lsbuf.st_mode & mask,
>  							entrymode & mask);
>  				}
>  				_alpm_log(handle, ALPM_LOG_DEBUG, "extract: skipping dir extraction of %s\n",
> -						entryname);
> +						filename);
>  				archive_read_data_skip(archive);
>  				return 0;
>  			} else {
>  				/* case 10/11: trying to overwrite dir with file/symlink, don't allow it */
>  				_alpm_log(handle, ALPM_LOG_ERROR, _("extract: not overwriting dir with file %s\n"),
> -						entryname);
> +						filename);
>  				archive_read_data_skip(archive);
>  				return 1;
>  			}
> @@ -239,20 +239,20 @@ static int extract_single_file(alpm_handle_t *handle, struct archive *archive,
>  			if(S_ISDIR(sbuf.st_mode)) {
>  				/* the symlink on FS is to a directory, so we'll use it */
>  				_alpm_log(handle, ALPM_LOG_DEBUG, "extract: skipping symlink overwrite of %s\n",
> -						entryname);
> +						filename);
>  				archive_read_data_skip(archive);
>  				return 0;
>  			} else {
>  				/* this is BAD. symlink was not to a directory */
>  				_alpm_log(handle, ALPM_LOG_ERROR, _("extract: symlink %s does not point to dir\n"),
> -						entryname);
> +						filename);
>  				archive_read_data_skip(archive);
>  				return 1;
>  			}
>  		} else if(S_ISREG(lsbuf.st_mode) && S_ISDIR(entrymode)) {
>  			/* case 6: trying to overwrite file with dir */
>  			_alpm_log(handle, ALPM_LOG_DEBUG, "extract: overwriting file with dir %s\n",
> -					entryname);
> +					filename);
>  		} else if(S_ISREG(entrymode)) {
>  			/* case 4,7: */
>  			/* if file is in NoUpgrade, don't touch it */
>