[pacman-dev] [PATCH] Skip rename() on NULL destfile in curl_download_internal()

[Dave Reisner]

On Wed, Aug 17, 2011 at 04:45:35PM +0200, Lukas Fleischer wrote:
> Avoid a potential segfault that may occur if we use a temporary file and
> fail to build the destination file name from the effective URL.
> 
> Signed-off-by: Lukas Fleischer <archlinux at cryptocrack.de>
> ---
> Untested, sorry. Should be trivial enough not to break anything though.
> Hopefully.
> 
>  lib/libalpm/dload.c |   14 +++++++++-----
>  1 files changed, 9 insertions(+), 5 deletions(-)
> 
> diff --git a/lib/libalpm/dload.c b/lib/libalpm/dload.c
> index 731d807..5cbb6b4 100644
> --- a/lib/libalpm/dload.c
> +++ b/lib/libalpm/dload.c
> @@ -384,12 +384,16 @@ cleanup:
>  	}
>  
>  	if(ret == 0) {
> -		if(rename(tempfile, destfile)) {
> -			_alpm_log(handle, ALPM_LOG_ERROR, _("could not rename %s to %s (%s)\n"),
> -					tempfile, destfile, strerror(errno));
> -			ret = -1;
> +		if (destfile) {
> +			if(rename(tempfile, destfile)) {
> +				_alpm_log(handle, ALPM_LOG_ERROR, _("could not rename %s to %s (%s)\n"),
> +						tempfile, destfile, strerror(errno));
> +				ret = -1;
> +			} else if(final_file) {
> +				*final_file = strdup(strrchr(destfile, '/') + 1);
> +			}
>  		} else if(final_file) {
> -			*final_file = strdup(strrchr(destfile, '/') + 1);
> +			*final_file = strdup(strrchr(tempfile, '/') + 1);

If you're going to change this line, can you use the STRDUP macro like I
should have?

  STRDUP(final_file, tempfile, RET_ERR(handle, ALPM_ERR_MEMORY, -1));

Same for the added strdup inside the if(destfile) block. Looks fine
otherwise.

>  		}
>  	}
>  
> -- 
> 1.7.6
> 
>