On Mon, 23 Aug 2021 at 14:56, Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> wrote:
>
> A proposed replacement of cf/fetch-set-upstream-while-detached which
> as noted in What's Cooking has been languishing for a while. I changed
> the die() in my version to a warning() as suggested by Junio & updated
> the test and commit message accordingly.
Thank you Ævar, for taking care of that and adding tests and my
apologizes for not responding in a timely manner.
> builtin/fetch.c | 11 +++++++++++
> t/t5553-set-upstream.sh | 22 ++++++++++++++++++++++
> 2 files changed, 33 insertions(+)
>
> diff --git a/builtin/fetch.c b/builtin/fetch.c
> index 25740c13df1..ca487edd805 100644
> --- a/builtin/fetch.c
> +++ b/builtin/fetch.c
> @@ -1623,6 +1623,17 @@ static int do_fetch(struct transport *transport,
> }
> }
> if (source_ref) {
> + if (!branch) {
> + const char *shortname = NULL;
> + if (!skip_prefix(source_ref->name,
> + "refs/heads/", &shortname))
> + shortname = source_ref->name;
Is skip_prefix ever able to fail?
- If yes, then shortname will be left pointing to NULL when we print
the warning below. Warning probably won't die because of NULL, but it
still will print something weird.
- If no, then an assert(!skip_prefix(source_ref->name, "refs/heads/",
&shortname)) might be more idiomatic.
> diff --git a/t/t5553-set-upstream.sh b/t/t5553-set-upstream.sh
> index b1d614ce18c..7d12ceff702 100755
> --- a/t/t5553-set-upstream.sh
> +++ b/t/t5553-set-upstream.sh
Thank for you adding tests. LGTM.
Regards,
--
Fruhwirth Clemens http://clemens.endorphin.org
