Re: [PATCH v2] pull, fetch: fix segfault in --set-upstream option
From: Clemens Fruhwirth <hidden>
Date: 2021-08-24 07:31:13
On Mon, 23 Aug 2021 at 14:56, Ævar Arnfjörð Bjarmason [off-list ref] 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.
quoted hunk ↗ jump to hunk
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.
quoted hunk ↗ jump to hunk
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