Re: [PATCH v4] for-each-ref: Always check stat_tracking_info()'s return value.
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jan 6, 2015, 23:51 UTC
- Message-ID
- <xmqqsifnmpsn.fsf@gitster.dls.corp.google.com>
- In-Reply-To
- <1420451935-6008-1-git-send-email-raphael.kubo.da.costa@intel.com>
Raphael Kubo da Costa <raphael.kubo.da.costa@intel.com> writes:
Show 30 quoted lines
> The code handling %(upstream:track) and %(upstream:trackshort) assumed
> it always had a valid branch that had been sanitized earlier in
> populate_value(), and thus did not check the return value of the call to
> stat_tracking_info().
>
> While there is indeed some sanitization code that basically corresponds
> to stat_tracking_info() returning 0 (no base branch set), the function
> can also return -1 when the base branch did exist but has since then
> been deleted.
>
> In this case, num_ours and num_theirs had undefined values and a call to
> `git for-each-ref --format="%(upstream:track)"` could print spurious
> values such as
>
> [behind -111794512]
> [ahead 38881640, behind 5103867]
>
> even for repositories with one single commit.
>
> We now properly verify stat_tracking_info()'s return value and do not
> print anything if it returns -1. This behavior also matches the
> documentation ("has no effect if the ref does not have tracking
> information associated with it").
>
> Helped-by: Eric Sunshine <sunshine@sunshineco.com>
> Helped-by: Jeff King <peff@peff.net>
> Signed-off-by: Raphael Kubo da Costa <raphael.kubo.da.costa@intel.com>
> ---
> v4: Use Jeff's suggestion and simplify the test case and making it
> easier to understand what is actually being tested.Nicely done, indeed. Thanks all. Will queue with some cosmetic tweaks.