Subject: Re: [PATCH] remote: initialize values that might not be set

From: Junio C Hamano <[email-masked]>
Date: Fri, 11 Jun 2021 10:41:11 +0900

Johannes Schindelin <[email-masked]> writes:

> You will notice that there are two Trace2 calls in that conditional `abf
> == AHEAD_BEHIND_FULL` block.

Yes, the calls use ours/theirs uninitialized.  Is it sensible to
show 0 there, or "(unset)" or its moral equivalent (e.g. "-1")?  Not
showing them indeed is an option, which is what you did below, and
that I find sensible, too.

> Now, what I failed to realize when reviewing this code (and I _bet_ Stolee
> was in the same boat when they contributed the patch) is that this version
> of `format_tracking_info()` is different from what is in v2.32.0. It is
> the version we have in the `microsoft/git` fork, and it has not yet made
> it upstream. To be precise, it is this commit:
> https://github.com/microsoft/git/commit/91209e591b0398c8334a78001a245807f7eb348a
>
> In light of this, it might make more sense for us to fixup! this commit
> thusly:
>
> -- snip --
> diff --git a/remote.c b/remote.c
> index caed9cbc31b1..cfb7b6bd8d30 100644
> --- a/remote.c
> +++ b/remote.c
> @@ -2110,7 +2110,7 @@ int format_tracking_info(struct branch *branch, struct strbuf *sb,
>  	sti = stat_tracking_info(branch, &ours, &theirs, &full_base, 0, abf);
>  	trace2_data_intmax("tracking", NULL, "stat_tracking_info/ab_flags", abf);
>  	trace2_data_intmax("tracking", NULL, "stat_tracking_info/ab_result", sti);
> -	if (abf == AHEAD_BEHIND_FULL) {
> +	if (sti >= 0 && abf == AHEAD_BEHIND_FULL) {
>  	    trace2_data_intmax("tracking", NULL, "stat_tracking_info/ab_ahead", ours);
>  	    trace2_data_intmax("tracking", NULL, "stat_tracking_info/ab_behind", theirs);
>  	}