On Wed, Feb 18, 2009 at 07:14:59PM +0100, Johannes Schindelin wrote:
> 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 <johannes.schindelin@xxxxxx>
> ---
>
> 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?
If the latter, isn't that an indication that this code path was not
being exercised by the test suite and it should be?
Now if only we had a way of measuring test coverage...
> --- 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 in
diff --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;
print_ref_item(&item, ref_list.maxwidth, verbose, abbrev, 1, "");
free(item.name);
}
Other than that, patch looks obviously correct (and I did a quick scan
to see that there were no other locations).
-Peff
--
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
