Re: [PATCH v2] pull, fetch: fix segfault in --set-upstream option
- From
- Clemens Fruhwirth <clemens@endorphin.org>
- Date
- Aug 24, 2021, 07:30 UTC
- Message-ID
- <CAG6gW_s6FHG6muJ6Hai-Sb2ntmygLymQZr8YrN212_SV1VhBAw@mail.gmail.com>
- In-Reply-To
- <patch-v2-1.1-9e846b76959-20210823T125434Z-avarab@gmail.com>
On Mon, 23 Aug 2021 at 14:56, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:
Show 5 quoted lines
> > 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.
Show 17 quoted lines
> 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