Re: What's cooking in git.git (May 2013, #09; Wed, 29)

Junio C Hamano [off-list ref] writes:

* rr/die-on-missing-upstream (2013-05-22) 2 commits
 - sha1_name: fix error message for @{<N>}, @{<date>}
 - sha1_name: fix error message for @{u}

 When a reflog notation is used for implicit "current branch", we
 did not say which branch and worse said "branch ''".

 Will merge to 'next'.

This interacts with the tests from

* fc/at-head (2013-05-08) 13 commits

to fail valgrind in t1508 like so:

  ==22927== Invalid read of size 1
  ==22927==    at 0x4C2C71B: __GI_strnlen (mc_replace_strmem.c:377)
  ==22927==    by 0x567FF6B: vfprintf (in /lib64/libc-2.17.so)
  ==22927==    by 0x56AC194: vsnprintf (in /lib64/libc-2.17.so)
  ==22927==    by 0x4EAC39: vreportf (usage.c:12)
  ==22927==    by 0x4EACA4: die_builtin (usage.c:36)
  ==22927==    by 0x4EAEE0: die (usage.c:103)
  ==22927==    by 0x4D8C51: get_sha1_1 (sha1_name.c:542)
  ==22927==    by 0x4D8E5D: get_sha1_with_context_1 (sha1_name.c:1299)
  ==22927==    by 0x4D992A: get_sha1_with_context (sha1_name.c:1417)
  ==22927==    by 0x4D99E1: get_sha1 (sha1_name.c:1124)
  ==22927==    by 0x45E1AC: cmd_rev_parse (rev-parse.c:761)
  ==22927==    by 0x4051B3: handle_internal_command (git.c:284)
  ==22927==  Address 0x5bebccb is 11 bytes inside a block of size 22 free'd
  ==22927==    at 0x4C2ACDA: free (vg_replace_malloc.c:468)
  ==22927==    by 0x4D8C37: get_sha1_1 (sha1_name.c:541)
  ==22927==    by 0x4D8E5D: get_sha1_with_context_1 (sha1_name.c:1299)
  ==22927==    by 0x4D992A: get_sha1_with_context (sha1_name.c:1417)
  ==22927==    by 0x4D99E1: get_sha1 (sha1_name.c:1124)
  ==22927==    by 0x45E1AC: cmd_rev_parse (rev-parse.c:761)
  ==22927==    by 0x4051B3: handle_internal_command (git.c:284)
  ==22927==    by 0x4053E7: main (git.c:492)

I think it's enough to squash this little change; leaking some memory
immediately before die() is not too bad, especially if we're going to
pass real_ref+11 into die()...

diff --git i/sha1_name.c w/sha1_name.c
index 5ea16ff..a07558d 100644
--- i/sha1_name.c
+++ w/sha1_name.c

@@ -538,7 +538,6 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1)
 					"back to %s.", len, str,
 					show_date(co_time, co_tz, DATE_RFC2822));
 			else {
-				free(real_ref);
 				die("Log for '%.*s' only has %d entries.",
 				    len, str, co_cnt);
 			}

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

Thomas Rast wrote:

Ah, good catch.  Thanks.

Sorry I didn't notice this while writing the patch.

Thomas Rast [off-list ref] writes:

This interacts with the tests from

quoted

* fc/at-head (2013-05-08) 13 commits

to fail valgrind in t1508 like so:

  ==22927== Invalid read of size 1
  ==22927==    at 0x4C2C71B: __GI_strnlen (mc_replace_strmem.c:377)
  ==22927==    by 0x567FF6B: vfprintf (in /lib64/libc-2.17.so)
  ==22927==    by 0x56AC194: vsnprintf (in /lib64/libc-2.17.so)
  ==22927==    by 0x4EAC39: vreportf (usage.c:12)
  ==22927==    by 0x4EACA4: die_builtin (usage.c:36)
  ==22927==    by 0x4EAEE0: die (usage.c:103)
  ==22927==    by 0x4D8C51: get_sha1_1 (sha1_name.c:542)
  ==22927==    by 0x4D8E5D: get_sha1_with_context_1 (sha1_name.c:1299)
  ==22927==    by 0x4D992A: get_sha1_with_context (sha1_name.c:1417)
  ==22927==    by 0x4D99E1: get_sha1 (sha1_name.c:1124)
  ==22927==    by 0x45E1AC: cmd_rev_parse (rev-parse.c:761)
  ==22927==    by 0x4051B3: handle_internal_command (git.c:284)
  ==22927==  Address 0x5bebccb is 11 bytes inside a block of size 22 free'd
  ==22927==    at 0x4C2ACDA: free (vg_replace_malloc.c:468)
  ==22927==    by 0x4D8C37: get_sha1_1 (sha1_name.c:541)
  ==22927==    by 0x4D8E5D: get_sha1_with_context_1 (sha1_name.c:1299)
  ==22927==    by 0x4D992A: get_sha1_with_context (sha1_name.c:1417)
  ==22927==    by 0x4D99E1: get_sha1 (sha1_name.c:1124)
  ==22927==    by 0x45E1AC: cmd_rev_parse (rev-parse.c:761)
  ==22927==    by 0x4051B3: handle_internal_command (git.c:284)
  ==22927==    by 0x4053E7: main (git.c:492)

I think it's enough to squash this little change; leaking some memory
immediately before die() is not too bad, especially if we're going to
pass real_ref+11 into die()...

Good catch, thanks.  when !len and real_ref is the current branch,
str just points into real_ref that is geting freed.