[PATCH 1/2] remote.c: don't leak the base branch name in format_tracking_info

Found by scan.coverity.com (Id: 1127809)

Signed-off-by: Stefan Beller <redacted>
---
 remote.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/remote.c b/remote.c
index 3d6c86a..2c1458f 100644
--- a/remote.c
+++ b/remote.c

@@ -1983,6 +1983,7 @@ int format_tracking_info(struct branch *branch, struct strbuf *sb)
 			strbuf_addf(sb,
 				_("  (use \"git pull\" to merge the remote branch into yours)\n"));
 	}
+	free(base);
 	return 1;
 }
 

-- 
2.1.0.rc2

Free the refspec.
Found by scan.coverity.com (Id: 1127806)

Signed-off-by: Stefan Beller <redacted>
---
 builtin/clone.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/builtin/clone.c b/builtin/clone.c
index bbd169c..dd4092b 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c

@@ -1004,5 +1004,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 	strbuf_release(&key);
 	strbuf_release(&value);
 	junk_mode = JUNK_LEAVE_ALL;
+
+	free(refspec);
 	return err;
 }

-- 
2.1.0.rc2

On 10.08.2014 15:57, Stefan Beller wrote:

Upon testing this one again, I get a warning
remote.c: In function ‘format_tracking_info’:
remote.c:1986:2: warning: passing argument 1 of ‘free’ discards ‘const’ qualifier from pointer target type [enabled by default]
  free(base);
  ^
In file included from git-compat-util.h:103:0,
                 from cache.h:4,
                 from remote.c:1:
/usr/include/stdlib.h:483:13: note: expected ‘void *’ but argument is of type ‘const char *’
 extern void free (void *__ptr) __THROW;
             ^

Please ignore this patch.

On Sun, Aug 10, 2014 at 05:14:33PM +0200, Stefan Beller wrote:

On 10.08.2014 15:57, Stefan Beller wrote:

quoted

Found by scan.coverity.com (Id: 1127809)

Signed-off-by: Stefan Beller <redacted>
---
 remote.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/remote.c b/remote.c
index 3d6c86a..2c1458f 100644
--- a/remote.c
+++ b/remote.c

@@ -1983,6 +1983,7 @@ int format_tracking_info(struct branch *branch, struct strbuf *sb)
 			strbuf_addf(sb,
 				_("  (use \"git pull\" to merge the remote branch into yours)\n"));
 	}
+	free(base);
 	return 1;
 }
 

Upon testing this one again, I get a warning
remote.c: In function ‘format_tracking_info’:
remote.c:1986:2: warning: passing argument 1 of ‘free’ discards ‘const’ qualifier from pointer target type [enabled by default]
  free(base);
  ^
In file included from git-compat-util.h:103:0,
                 from cache.h:4,
                 from remote.c:1:
/usr/include/stdlib.h:483:13: note: expected ‘void *’ but argument is of type ‘const char *’
 extern void free (void *__ptr) __THROW;
             ^

Please ignore this patch.

I think your patch is definitely fixing a leak; it's just that the
existing code is a little sloppy. It does:

  const char *base;
  ...
  base = branch->merge[0]->dst;
  base = shorten_unambiguous_ref(base, 0);

In the first assignment, "base" should be const, as we are pointing to
somebody else's memory. But in the second, we use the same pointer to
store newly allocated memory from shorten_unambiguous_ref.

In the general case, you need two pointers to do this right. However, we
don't actually look at "base" between the two assignments, so I think
you could just do it as:

  char *base;
  ...
  base = shorten_unambiguous_ref(branch->merge[0]->dst, 0);

-Peff

Found by scan.coverity.com (Id: 1127809)

Signed-off-by: Stefan Beller <redacted>
---
 remote.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/remote.c b/remote.c
index 3d6c86a..894db09 100644
--- a/remote.c
+++ b/remote.c

@@ -1920,7 +1920,7 @@ int stat_tracking_info(struct branch *branch, int *num_ours, int *num_theirs)
 int format_tracking_info(struct branch *branch, struct strbuf *sb)
 {
 	int ours, theirs;
-	const char *base;
+	char *base;
 	int upstream_is_gone = 0;
 
 	switch (stat_tracking_info(branch, &ours, &theirs)) {

@@ -1936,8 +1936,7 @@ int format_tracking_info(struct branch *branch, struct strbuf *sb)
 		break;
 	}
 
-	base = branch->merge[0]->dst;
-	base = shorten_unambiguous_ref(base, 0);
+	base = shorten_unambiguous_ref(branch->merge[0]->dst, 0);
 	if (upstream_is_gone) {
 		strbuf_addf(sb,
 			_("Your branch is based on '%s', but the upstream is gone.\n"),

@@ -1983,6 +1982,7 @@ int format_tracking_info(struct branch *branch, struct strbuf *sb)
 			strbuf_addf(sb,
 				_("  (use \"git pull\" to merge the remote branch into yours)\n"));
 	}
+	free(base);
 	return 1;
 }
 

-- 
2.1.0.rc2