Re: [PATCH v5 1/2] branch: accept multiple upstream branches for tracking
- From
Glen Choo <chooglen@google.com>
- Date
- Dec 8, 2021, 23:53 UTC
- Message-ID
- <kl6lr1amac23.fsf@chooglen-macbookpro.roam.corp.google.com>
- In-Reply-To
- <ba7d557725e70f2ae8f10ae5992c8168eb97f2fc.1638859949.git.steadmon@google.com>
I noticed some test failures due to the printf_ln(msg_fmt) region. Since you are reworking this, the problem might just go away, but I thought I should mention it just in case.
Show 11 quoted lines
> + if (all_shortnames && origin) {
> + if (rebasing && plural)
> + msg_fmt = "Branch '%s' set up to track remote branches %s from '%s' by rebasing.";
> + else if (rebasing && !plural)
> + msg_fmt = "Branch '%s' set up to track remote branch %s from '%s' by rebasing.";
> + else if (!rebasing && plural)
> + msg_fmt = "Branch '%s' set up to track remote branches %s from '%s'.";
> + else if (!rebasing && !plural)
> + msg_fmt = "Branch '%s' set up to track remote branch %s from '%s'.";
> +
> + printf_ln(_(msg_fmt), local, ref_string, origin);Here
Show 37 quoted lines
> } else {
> - if (origin)
> - printf_ln(rebasing ?
> - _("Branch '%s' set up to track remote ref '%s' by rebasing.") :
> - _("Branch '%s' set up to track remote ref '%s'."),
> - local, remote);
> - else
> - printf_ln(rebasing ?
> - _("Branch '%s' set up to track local ref '%s' by rebasing.") :
> - _("Branch '%s' set up to track local ref '%s'."),
> - local, remote);
> + if (all_shortnames && !origin && rebasing && plural)
> + msg_fmt = "Branch '%s' set up to track local branches %s by rebasing.";
> + if (all_shortnames && !origin && rebasing && !plural)
> + msg_fmt = "Branch '%s' set up to track local branch %s by rebasing.";
> + if (all_shortnames && !origin && !rebasing && plural)
> + msg_fmt = "Branch '%s' set up to track local branches %s.";
> + if (all_shortnames && !origin && !rebasing && !plural)
> + msg_fmt = "Branch '%s' set up to track local branch %s.";
> + if (!all_shortnames && origin && rebasing && plural)
> + msg_fmt = "Branch '%s' set up to track remote refs %s by rebasing.";
> + if (!all_shortnames && origin && rebasing && !plural)
> + msg_fmt = "Branch '%s' set up to track remote ref %s by rebasing.";
> + if (!all_shortnames && origin && !rebasing && plural)
> + msg_fmt = "Branch '%s' set up to track remote refs %s.";
> + if (!all_shortnames && origin && !rebasing && !plural)
> + msg_fmt = "Branch '%s' set up to track remote ref %s.";
> + if (!all_shortnames && !origin && rebasing && plural)
> + msg_fmt = "Branch '%s' set up to track local refs %s by rebasing.";
> + if (!all_shortnames && !origin && rebasing && !plural)
> + msg_fmt = "Branch '%s' set up to track local ref %s by rebasing.";
> + if (!all_shortnames && !origin && !rebasing && plural)
> + msg_fmt = "Branch '%s' set up to track local refs %s.";
> + if (!all_shortnames && !origin && !rebasing && !plural)
> + msg_fmt = "Branch '%s' set up to track local ref %s.";
> +
> + printf_ln(_(msg_fmt), local, ref_string);and here
> } > + > + strbuf_release(&ref_string); > }
We print ref_string, which is a strbuf. This causes t/t3200-branch.sh to segfault on my mac + clang, but inconsistently! With -O2, it doesn't always segfault, but the wrong memory is read:
Branch 'my3' set up to track remote branch local from 'Branch '%s' set up to track remote branch %s from '%s'.'.
With -O0, it always segfaults.
You can see this in the osx-clang run in [1], but it looks like the gcc ones refuse to build.
s/ref_string/ref_string.buf should fix the problem.
[1] https://github.com/chooglen/git/runs/4464134763?check_suite_focus=true