Re: [PATCH] bisect: display first bad commit without forking a new process

[Christian Couder <chriscool@tuxfamily.org>]

Le Thursday 28 May 2009, Andreas Ericsson a écrit :
> Junio C Hamano wrote:
> > Christian Couder <chriscool@tuxfamily.org> writes:
> >> Le Wednesday 27 May 2009, Christian Couder a écrit :
> >>> Previously "git diff-tree --pretty COMMIT" was run using
> >>> "run_command_v_opt" to display information about the first bad
> >>> commit.
> >>>
> >>> The goal of this patch is to avoid a "fork" and an "exec" call
> >>> when displaying that information.
> >>>
> >>> To do that, we manually setup revision information as
> >>> "git diff-tree --pretty" would do it, and then use the
> >>> "log_tree_commit" function.
> >>>
> >>> Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
> >>> ---
> >>>  bisect.c |   30 +++++++++++++++++++++++++++---
> >>>  1 files changed, 27 insertions(+), 3 deletions(-)
> >>>
> >>> diff --git a/bisect.c b/bisect.c
> >>> index c43c120..e94a77b 100644
> >>> --- a/bisect.c
> >>> +++ b/bisect.c
> >>> @@ -816,6 +816,31 @@ static void
> >>> check_good_are_ancestors_of_bad(const char *prefix) }
> >>>
> >>>  /*
> >>> + * This does "git diff-tree --pretty COMMIT" without one fork+exec.
> >>> + */
> >>> +static void show_diff_tree(const char *prefix, struct commit
> >>> *commit) +{
> >>> +	static struct rev_info opt;
> >>
> >> Oops, "static" can be removed, it's a copy-paste error, sorry.
> >
> > Is that "can" or "must"?  If the answer is the latter, shouldn't the
> > function be renamed to make it clear it is a bisect specific thing?
>
> I think he was referring to the 'static' in the variable declarations.

You are right. I will send a v2 of this patch anyway.

Thanks for this clarification,
Christian.