git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Michael Haggerty <mhagger@alum.mit.edu>
Date
May 31, 2016, 04:25 UTC
Message-ID
<574D122F.7080608@alum.mit.edu>
In-Reply-To
<20160530182148.18801-1-pranit.bauva@gmail.com>
On 05/30/2016 08:21 PM, Pranit Bauva wrote:
Show 18 quoted lines
> 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

> [...]
Michael
Previous: Pranit BauvaNext: Pranit Bauva
Message 4 of 5 in “bisect--helper: `bisect_clean_state` shell function in C”
  1. bisect--helper: `bisect_clean_state` shell function in CPranit Bauva, May 30, 2016
  2. Christian CouderMay 30, 2016
  3. Pranit BauvaMay 31, 2016
  4. Michael HaggertyMay 31, 2016
  5. Pranit BauvaMay 31, 2016

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.