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

[Dave Reisner]

On Wed, Aug 17, 2011 at 11:34:19AM -0400, Dave Reisner wrote:
> 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.
> 
> >  		}
> >  	}

Note that this doesn't actually solve the issue of a zero length
filename...

 ┌┤noclaf at rampage:1 bugfix ~/src/c/pacman
 └╼ sudo ./src/pacman/pacman -U http://www.archlinux.org/
 alpmtmp.sj9IKg           23.2K  443.7K/s 00:00:00 [-----------------------] 100%
error: could not rename /mnt/Haven/packages/alpmtmp.sj9IKg to /mnt/Haven/packages/ (Not a directory)
warning: failed to download http://www.archlinux.org/
error: 'http://www.archlinux.org/': unexpected error