On Wed, Aug 17, 2011 at 09:40:22AM -0400, Dave Reisner wrote: > On Wed, Aug 17, 2011 at 10:15:17AM +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 <archli...@cryptocrack.de> > > --- > > Another approach is to use a random default destfile name. Since the URL > > parsing is very unlikely to fail, I took the easy option here. > > > > lib/libalpm/dload.c | 5 +++++ > > 1 files changed, 5 insertions(+), 0 deletions(-) > > > > diff --git a/lib/libalpm/dload.c b/lib/libalpm/dload.c > > index 731d807..c552d2b 100644 > > --- a/lib/libalpm/dload.c > > +++ b/lib/libalpm/dload.c > > @@ -373,6 +373,11 @@ static int curl_download_internal(struct dload_payload > > *payload, > > destfile = get_fullpath(localpath, > > effective_filename, ""); > > } > > } > > + else { > > + _alpm_log(handle, ALPM_LOG_ERROR, _("could not parse > > file name from url (%s)"), > > + effective_url); > > + goto cleanup; > > + } > > } > > > > ret = 0; > > -- > > 1.7.6 > > > > > > This is after the file has already been downloaded. We shouldn't bail > out so easily since we already have the file. It looks like in this > case, we wouldn't even unlink after jumping to cleanup.
Oh, right. I missed that part. > > I'm not sure that this is the right thing to do. Worst case scenario, we > should probably just skip the rename. Yeah, either use some default for destfile (some random name or just set "destfile = strdup(tempfile);") or skip the rename (I prefer the latter).