Re: [RFC/PATCH] bisect--helper: `bisect_clean_state` shell function in C

> On 05/30/2016 08:21 PM, Pranit Bauva wrote:
>> 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 .
>>
>> Using `bisect_clean_state` subcommand is a measure to port shell
>> function to C so as to use the existing test suite. As more functions
>> are ported, this subcommand will be retired and will be called by
>> bisect_reset() and bisect_start().
>>
>> Mentored-by: Lars Schneider <larsxschneider@gmail.com>
>> Mentored-by: Christian Couder <chriscool@tuxfamily.org>
>> Signed-off-by: Pranit Bauva <pranit.bauva@gmail.com>
>> ---
>> This patch contains a bug. I have tried to identify the bug and I suppose it
>> exists in do_for_each_entry_in_dir(). I have reproduced the debugging session
>> at this link[1]. I have seen that some patches in mailing list regarding
>> iterating over refs. Will those affect this? Or is this bug fixed in those
>> patches?
>
> The problem is that it is not legal to modify references while iterating
> over them. See [1]. Your remove_bisect_ref() callback function deletes
> references, which modifies the reference cache that is being iterated over.

> Instead I suggest that your remove_bisect_ref() add the references to a
> string_list, then call delete_refs() *after* the iteration is over.
> Alternatively, you can change remove_bisect_ref() to call
> ref_transaction_delete() to add reference deletions to a
> ref_transaction, then call ref_transaction_commit() after the iteration
> is over. See the rm() function in builtin/remote.c [2] for an example.

> [1]
> https://github.com/git/git/blob/f3913c2d03abc660140678a9e14dac399f847647/refs.h#L176-L184
> [2]
> https://github.com/git/git/blob/f3913c2d03abc660140678a9e14dac399f847647/builtin/remote.c#L738
>
>> [...]
>