All of lore.kernel.org
 help / color / mirror / Atom feed
From: Michael Haggerty <mhagger@alum.mit.edu>
To: Pranit Bauva <pranit.bauva@gmail.com>, git@vger.kernel.org
Cc: larsxschneider@gmail.com, chriscool@tuxfamily.org,
	christian.couder@gmail.com, peff@peff.net
Subject: Re: [RFC/PATCH] bisect--helper: `bisect_clean_state` shell function in C
Date: Tue, 31 May 2016 06:25:19 +0200	[thread overview]
Message-ID: <574D122F.7080608@alum.mit.edu> (raw)
In-Reply-To: <20160530182148.18801-1-pranit.bauva@gmail.com>

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

> [...]

Michael

  parent reply	other threads:[~2016-05-31  4:25 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-05-30 18:21 [RFC/PATCH] bisect--helper: `bisect_clean_state` shell function in C Pranit Bauva
2016-05-30 21:48 ` Christian Couder
2016-05-31  6:33   ` Pranit Bauva
2016-05-31  4:25 ` Michael Haggerty [this message]
2016-05-31  6:40   ` Pranit Bauva

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=574D122F.7080608@alum.mit.edu \
    --to=mhagger@alum.mit.edu \
    --cc=chriscool@tuxfamily.org \
    --cc=christian.couder@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=larsxschneider@gmail.com \
    --cc=peff@peff.net \
    --cc=pranit.bauva@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.