Re: [PATCH] Avoid segfault with 'git branch' when the HEAD is detached
From: Johannes Schindelin <hidden>
Date: 2016-06-15 22:46:14
Hi, On Wed, 18 Feb 2009, Jeff King wrote:
On Wed, Feb 18, 2009 at 07:14:59PM +0100, Johannes Schindelin wrote:quoted
A recent addition to the ref_item struct was not taken care of, leading to a segmentation fault when accessing the (uninitialized) "dest" member. Signed-off-by: Johannes Schindelin <redacted> --- Unfortunately not found by valgrind.Meaning that the bug was created after your valgrind testing (which takes a painfully long time to run, and so only happens occasionally), and therefore you found it by hand? Or meaning that even running the test suite with valgrind did not reveal the problem?
It bit me. IOW I had to fix it before I could finish up the work for the day.
If the latter, isn't that an indication that this code path was not being exercised by the test suite and it should be?
Like I said, I had to finish up some work for the day, that's why I did not have time to add a test.
Now if only we had a way of measuring test coverage...
Yes, I also want the gcov series. Patience, grass hopper, patience: after 1.6.2.
quoted hunk ↗ jump to hunk
quoted
--- a/builtin-branch.c +++ b/builtin-branch.c@@ -441,7 +441,9 @@ static void print_ref_list(int kinds, int detached, int verbose, int abbrev, str is_descendant_of(head_commit, with_commit)) { struct ref_item item; item.name = xstrdup("(no branch)"); + item.len = strlen(item.name); item.kind = REF_LOCAL_BRANCH; + item.dest = NULL; item.commit = head_commit; if (strlen(item.name) > ref_list.maxwidth) ref_list.maxwidth = strlen(item.name);Maybe replace the repeated strlens below with item.len? I.e., squash indiff --git a/builtin-branch.c b/builtin-branch.c index 13e4de8..14d4b91 100644 --- a/builtin-branch.c +++ b/builtin-branch.c@@ -443,8 +443,8 @@ static void print_ref_list(int kinds, int detached, int verbose, int abbrev, str item.kind = REF_LOCAL_BRANCH; item.dest = NULL; item.commit = head_commit; - if (strlen(item.name) > ref_list.maxwidth) - ref_list.maxwidth = strlen(item.name); + if (item.len > ref_list.maxwidth) + ref_list.maxwidth = item.len;
Yeah, I did not think of that. I checked that there are no other instances where a member of ref_item was uninitialized, and that took already more time than I had. Ciao, Dscho