[pacman-dev] [BUG] segfault when the download fails

[Xavier]

On Thu, Dec 06, 2007 at 06:24:55PM -0600, Aaron Griffin wrote:
> On Dec 6, 2007 5:46 PM, Xavier <shiningxc at gmail.com> wrote:
> > [00:27:01] debug: using 'hunspell-1.2.1-2-i686.pkg.tar.gz'
> > [00:27:02] debug: using 'openoffice-fr-2.3.1-1-i686.pkg.tar.gz'
> > [00:27:02] debug: using 'hunspell-1.2.1-2-i686.pkg.tar.gz'
> 
> A cursory look here shows something important. We have the following logic:
> 
> for s in servers:
>     for f in files:
>         if cant_get_file: continue
> 
> Well that's just not right - we want to pop out to the parent loop,
> not move to the next file
> 
> While that's wrong and all, it's actually not really the error... I
> mean, just looking at it... it should still WORK in this case, just
> much slower due to extra loop iterations.
> 
> My guess? There's an alpm_list_free inside the "for s in servers" loop
> that should probably be OUTSIDE it, unless that's a totally different
> bug, but I'd expect the list_free that DOESN'T null out the pointer is
> going to re-add without reallocating and blow up like this.
> 

Oh of course, you are perfectly right. I saw that free, but didn't pay
attention that it was inside that loop ;)


>From c8dc427e07cbdfcba496f69de4ec524632295801 Mon Sep 17 00:00:00 2001
From: Chantry Xavier <shiningxc at gmail.com>
Date: Fri, 7 Dec 2007 01:51:14 +0100
Subject: [PATCH] libalpm/server.c : fix segfault when downloading failed.

The alpm_list_free(complete) needs to be done OUTSIDE the loop walking
through the server list.

Besides, when downloading fails, we might as well skip directly to the next
server, instead of trying to download other files from it. So
s/continue/break.

Signed-off-by: Chantry Xavier <shiningxc at gmail.com>
---
 lib/libalpm/server.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/libalpm/server.c b/lib/libalpm/server.c
index 8aa5a45..bd7752b 100644
--- a/lib/libalpm/server.c
+++ b/lib/libalpm/server.c
@@ -249,7 +249,7 @@ int _alpm_downloadfiles_forreal(alpm_list_t *servers, const char *localpath,
 					}
 					/* try the next server */
 					downloadFreeURL(fileurl);
-					continue;
+					break;
 				} else {
 						_alpm_log(PM_LOG_DEBUG, "connected to %s successfully\n", fileurl->host);
 				}
@@ -410,8 +410,8 @@ int _alpm_downloadfiles_forreal(alpm_list_t *servers, const char *localpath,
 		if(alpm_list_count(complete) == alpm_list_count(files)) {
 			done = 1;
 		}
-		alpm_list_free(complete);
 	}
+	alpm_list_free(complete);
 
 	return(done ? 0 : -1);
 }
-- 
1.5.3.7