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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hey Michael,

On Tue, May 31, 2016 at 9:55 AM, Michael Haggerty <mhagger@xxxxxxxxxxxx> wrote:
> 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@xxxxxxxxx>
>> Mentored-by: Christian Couder <chriscool@xxxxxxxxxxxxx>
>> Signed-off-by: Pranit Bauva <pranit.bauva@xxxxxxxxx>
>> ---
>> 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.

Thanks for explaining this. I wasn't aware about this.

> 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.

I have gone through the links and I think I will stick with the first
one because I don't see how the latter one would be better.

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

Regards,
Pranit Bauva
--
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



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]