Re: [PATCH v2 1/6] bisect--helper: `bisect_clean_state` shell function in C

Hey Eric,

On Wed, Jun 15, 2016 at 11:34 PM, Eric Sunshine [off-list ref] wrote:

On Wed, Jun 15, 2016 at 10:00 AM, Pranit Bauva [off-list ref] wrote:

quoted

Reimplement `bisect_clean_state` shell function in C and add a
`bisect-clean-state` subcommand to `git bisect--helper` to call it from
git-bisect.sh .
[...]
Signed-off-by: Pranit Bauva <redacted>
---

diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
+static int mark_for_removal(const char *refname, const struct object_id *oid,
+                           int flag, void *cb_data)
+{
+       struct string_list *refs = cb_data;
+       char *ref = xstrfmt("refs/bisect/%s", refname);
+       string_list_append(refs, ref);
+       return 0;
+}
+
+static int bisect_clean_state(void)
+{
+       int result = 0;
+
+       /* There may be some refs packed during bisection */
+       struct string_list refs_for_removal = STRING_LIST_INIT_NODUP;
+       for_each_ref_in("refs/bisect/", mark_for_removal, (void *) &refs_for_removal);
+       string_list_append(&refs_for_removal, xstrdup("BISECT_HEAD"));
+       result = delete_refs(&refs_for_removal);
+       string_list_clear(&refs_for_removal, 0);

This is leaking all the strings added to 'refs_for_removal', isn't it?
Either you need to loop over the items and free the strings manually,
or (if it's not too ugly), set 'strdup_strings' before invoking
string_list_clear().

I didn't carefully see that in the function string_list_clear() it
only free()'s the memory if strdup_strings is 1. I think changing
strdup_strings to 1 would be an easy way out but it would make the
code very ugly and non-trivial. On the other hand, I can initialize
the string as STRING_LIST_INIT_DUP which will automatically set
strdup_strings as 1 and then also free the memory of ref at that point
after the string ref was appended to the list. Personally, I will
prefer the latter one.

Regards,
Pranit Bauva