Re: [PATCH] * remote.c (valid_fetch_refspec): remove useless if-before-free test

[Brandon Casey <casey@xxxxxxxxxxxxxxx>]

Jim Meyering wrote:
> We removed a handful of these useless if-before-free tests
> several months ago.  This change removes a new one that snuck back in.

> diff --git a/remote.c b/remote.c
> index f61a3ab..105668f 100644
> --- a/remote.c
> +++ b/remote.c
> @@ -579,8 +579,7 @@ int valid_fetch_refspec(const char *fetch_refspec_str)
>  	struct refspec *refspec;
> 
>  	refspec = parse_refspec_internal(1, fetch_refspec, 1, 1);
> -	if (refspec)
> -		free(refspec);
> +	free(refspec);
>  	return !!refspec;
>  }

Maybe we should also begin the process of not leaking memory here...

diff --git a/remote.c b/remote.c
index 7f2897b..984ad1b 100644
--- a/remote.c
+++ b/remote.c
@@ -449,6 +449,20 @@ static int verify_refname(char *name, int is_glob)
	return result;
 }
 
+void free_refspecs(struct refspec *refspec, int nr_refspec)
+{
+	int i;
+
+	if (!refspec)
+		return;
+
+	for (i = 0; i < nr_refspec; i++) {
+		free(refspec[i].src);
+		free(refspec[i].dst);
+	}
+	free(refspec);
+}
+
 static struct refspec *parse_refspec_internal(int nr_refspec, const char **refspec, int fetch, int verify)
 {
 	int i;
@@ -567,7 +581,12 @@ static struct refspec *parse_refspec_internal(int nr_refspec, const char **refsp
 
  invalid:
 	if (verify) {
-		free(rs);
+		/*
+		 * nr_refspec must be greater than zero and i must be valid
+		 * since it is only possible to reach this point from within
+		 * the for loop above.
+		 */
+		free_refspecs(rs, i+1);
 		return NULL;
 	}
 	die("Invalid refspec '%s'", refspec[i]);
@@ -579,8 +598,7 @@ int valid_fetch_refspec(const char *fetch_refspec_str)
 	struct refspec *refspec;
 
 	refspec = parse_refspec_internal(1, fetch_refspec, 1, 1);
-	if (refspec)
-		free(refspec);
+	free_refspecs(refspec, 1);
 	return !!refspec;
 }
 
diff --git a/remote.h b/remote.h
index 091b1d0..2601f6e 100644
--- a/remote.h
+++ b/remote.h
@@ -78,6 +78,7 @@ void ref_remove_duplicates(struct ref *ref_map);
 int valid_fetch_refspec(const char *refspec);
 struct refspec *parse_fetch_refspec(int nr_refspec, const char **refspec);
 struct refspec *parse_push_refspec(int nr_refspec, const char **refspec);
+void free_refspecs(struct refspec *refspec, int nr_refspec);
 
 int match_refs(struct ref *src, struct ref *dst, struct ref ***dst_tail,
 	       int nr_refspec, const char **refspec, int all);
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html