From: Tay Ray Chuan <rctay89@gmail.com>
Date: Fri, 07 Oct 2011 06:49:12 GMT
Subject: Re: [PATCH] fetch: plug two leaks on error exit in store_updated_refs
Message-ID: <CALUzUxp4Eo7j=kM7YPJbj70-rwuyFK5V1mZZMY7vBwwPYWS6gQ@mail.gmail.com>
In-Reply-To: <4E8E98A7.8010008@lsrfire.ath.cx>

On Fri, Oct 7, 2011 at 2:13 PM, René Scharfe
<rene.scharfe@lsrfire.ath.cx> wrote:
> diff --git a/builtin/fetch.c b/builtin/fetch.c
> index 7a4e41c..79db796 100644
> --- a/builtin/fetch.c
> +++ b/builtin/fetch.c
> @@ -379,8 +379,12 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
>                url = xstrdup("foreign");
>
>        rm = ref_map;
> -       if (check_everything_connected(iterate_ref_map, 0, &rm))
> -               return error(_("%s did not send all necessary objects\n"), url);
> +       if (check_everything_connected(iterate_ref_map, 0, &rm)) {
> +               error(_("%s did not send all necessary objects\n"), url);
> +               free(url);
> +               fclose(fp);
> +               return -1;
> +       }
>
>        for (rm = ref_map; rm; rm = rm->next) {
>                struct ref *ref = NULL;
> --
> 1.7.7

How about reusing the function's cleanup calls, like this?

-- >8 --
diff --git a/builtin/fetch.c b/builtin/fetch.c
index fc254b6..56267c4 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -423,8 +423,10 @@ static int store_updated_refs(const char
*raw_url, const char *remote_name,
 	else
 		url = xstrdup("foreign");

-	if (check_everything_connected(ref_map, 0))
-		return error(_("%s did not send all necessary objects\n"), url);
+	if (check_everything_connected(ref_map, 0)) {
+		rc = error(_("%s did not send all necessary objects\n"), url);
+		goto abort;
+	}

 	for (rm = ref_map; rm; rm = rm->next) {
 		struct ref *ref = NULL;
@@ -506,12 +508,15 @@ static int store_updated_refs(const char
*raw_url, const char *remote_name,
 				fprintf(stderr, " %s\n", note);
 		}
 	}
-	free(url);
-	fclose(fp);
+
 	if (rc & STORE_REF_ERROR_DF_CONFLICT)
 		error(_("some local refs could not be updated; try running\n"
 		      " 'git remote prune %s' to remove any old, conflicting "
 		      "branches"), remote_name);
+
+abort:
+	free(url);
+	fclose(fp);
 	return rc;
 }

--

-- 
Cheers,
Ray Chuan