All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Cc: Derrick Stolee via GitGitGadget <gitgitgadget@gmail.com>,
	git@vger.kernel.org, stolee@gmail.com,
	Derrick Stolee <derrickstolee@github.com>,
	Derrick Stolee <dstolee@microsoft.com>
Subject: Re: [PATCH] remote: initialize values that might not be set
Date: Fri, 11 Jun 2021 10:41:11 +0900	[thread overview]
Message-ID: <xmqqeed95gaw.fsf@gitster.g> (raw)
In-Reply-To: <nycvar.QRO.7.76.6.2106101046470.57@tvgsbejvaqbjf.bet> (Johannes Schindelin's message of "Thu, 10 Jun 2021 11:24:52 +0200 (CEST)")

Johannes Schindelin <Johannes.Schindelin@gmx.de> 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);
>  	}

  reply	other threads:[~2021-06-11  1:41 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-07 12:39 [PATCH] remote: initialize values that might not be set Derrick Stolee via GitGitGadget
2021-06-07 21:59 ` Johannes Schindelin
2021-06-07 23:21   ` Junio C Hamano
2021-06-10  9:24     ` Johannes Schindelin
2021-06-11  1:41       ` Junio C Hamano [this message]
2021-06-11 17:56       ` Derrick Stolee

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=xmqqeed95gaw.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=derrickstolee@github.com \
    --cc=dstolee@microsoft.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=stolee@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.