Re: [PATCH] push: region_leave trace for negotiate_using_fetch

[Junio C Hamano <gitster@pobox.com>]

Sam Delmerico <delmerico@google.com> writes:

> There were two region_enter events for negotiate_using_fetch instead of
> one enter and one leave. This commit replaces the second region_enter
> event with a region_leave.
>
> Signed-off-by: Sam Delmerico <delmerico@google.com>
> ---
>  fetch-pack.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Looks right, after skimming a29263cf (fetch-pack: add tracing for
negotiation rounds, 2022-08-02).  Two questions and a half.

 * How did you find it?  Code inspection?  While writing a script to
   parse the output from around this area, your script noticed the
   ever-increasing nesting level?  Something else?

 * Would it be feasible to write some tests or tools that find
   similar problems (semi-)automatically?

 * Is the breakage (before this patch) something easily demonstrated
   in a new test in t/ somewhere?  And if so, would it be worth
   doing?

Thanks.  Will queue.


>
> diff --git a/fetch-pack.c b/fetch-pack.c
> index 31a72d43de..dba6d79944 100644
> --- a/fetch-pack.c
> +++ b/fetch-pack.c
> @@ -2218,7 +2218,7 @@ void negotiate_using_fetch(const struct oid_array *negotiation_tips,
>  					   the_repository, "%d",
>  					   negotiation_round);
>  	}
> -	trace2_region_enter("fetch-pack", "negotiate_using_fetch", the_repository);
> +	trace2_region_leave("fetch-pack", "negotiate_using_fetch", the_repository);
>  	trace2_data_intmax("negotiate_using_fetch", the_repository,
>  			   "total_rounds", negotiation_round);
>  	clear_common_flag(acked_commits);
>
> base-commit: a26002b62827b89a19b1084bd75d9371d565d03c