# [PATCH] fetch: Fix segfault on pull --set-upstream outside a branch. 5 messages from 2021-07-06 to 2021-07-06. Participants: Clemens Fruhwirth, Junio C Hamano, Eric Sunshine. Thread: https://gitlist.dev/t/56056 ## Clemens Fruhwirth, 2021-07-06 16:22 Subject: [PATCH] fetch: Fix segfault on pull --set-upstream outside a branch. Message-ID: <20210706162238.575988-1-clemens@endorphin.org> URL: https://gitlist.dev/e/20210706162238.575988-1-clemens%40endorphin.org ``` branch_get("HEAD") can return NULL, when we are outside a branch and the user calls pull --set-upstream. Catch this case and warn the user to avoid a segfault. Signed-off-by: Clemens Fruhwirth --- builtin/fetch.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/builtin/fetch.c b/builtin/fetch.c index 9191620e50..1097235b90 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -1602,6 +1602,10 @@ static int do_fetch(struct transport *transport, struct ref *rm; struct ref *source_ref = NULL; + if (!branch) { + warning(_("no branch detected to use --set-upstream with.")); + goto skip; + } /* * We're setting the upstream configuration for the * current branch. The relevant upstream is the -- 2.31.1 ``` ## Junio C Hamano, 2021-07-06 17:57 Subject: Re: [PATCH] fetch: Fix segfault on pull --set-upstream outside a branch. Message-ID: URL: https://gitlist.dev/e/xmqq1r8bl40i.fsf%40gitster.g In-Reply-To: <20210706162238.575988-1-clemens@endorphin.org> ``` Clemens Fruhwirth writes: > branch_get("HEAD") can return NULL, when we are outside a branch and > the user calls pull --set-upstream. Catch this case and warn the user > to avoid a segfault. Nit. What do you mean by "outside a branch"? If you mean "when the HEAD is detached", please say so, and use the same terminology in your warning message. Thanks. > > Signed-off-by: Clemens Fruhwirth > --- > builtin/fetch.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/builtin/fetch.c b/builtin/fetch.c > index 9191620e50..1097235b90 100644 > --- a/builtin/fetch.c > +++ b/builtin/fetch.c > @@ -1602,6 +1602,10 @@ static int do_fetch(struct transport *transport, > struct ref *rm; > struct ref *source_ref = NULL; > > + if (!branch) { > + warning(_("no branch detected to use --set-upstream with.")); > + goto skip; > + } > /* > * We're setting the upstream configuration for the > * current branch. The relevant upstream is the ``` ## Eric Sunshine, 2021-07-06 18:03 Subject: Re: [PATCH] fetch: Fix segfault on pull --set-upstream outside a branch. Message-ID: URL: https://gitlist.dev/e/CAPig%2BcTRXTGe-MNTy%3D2gk1eX8G%2B0fa303nrLnEtX1uHUC2usmg%40mail.gmail.com In-Reply-To: ``` On Tue, Jul 6, 2021 at 1:57 PM Junio C Hamano wrote: > Clemens Fruhwirth writes: > > > branch_get("HEAD") can return NULL, when we are outside a branch and > > the user calls pull --set-upstream. Catch this case and warn the user > > to avoid a segfault. > > Nit. What do you mean by "outside a branch"? If you mean "when the > HEAD is detached", please say so, and use the same terminology in > your warning message. Also, adding a new test, probably to `t/t5553-set-upstream.sh`, would be welcome. The test can likely be as simple as ensuring that the command does not crash, and perhaps check that the outcome was as expected. ``` ## Junio C Hamano, 2021-07-06 19:48 Subject: Re: [PATCH] fetch: Fix segfault on pull --set-upstream outside a branch. Message-ID: URL: https://gitlist.dev/e/xmqqzguzi5q7.fsf%40gitster.g In-Reply-To: ``` Junio C Hamano writes: > Nit. What do you mean by "outside a branch"? If you mean "when the > HEAD is detached", please say so, and use the same terminology in > your warning message. Here is what I queued for today's integration run. Eric's suggestion to add a simple test is a good idea, too. Thanks. -- >8 -- From: Clemens Fruhwirth Date: Tue, 6 Jul 2021 18:22:38 +0200 Subject: [PATCH] fetch: fix segfault on pull --set-upstream while on a detached HEAD branch_get("HEAD") can return NULL, when HEAD is detached, and cause the code to segfault when the user calls "pull --set-upstream". Catch this case and warn the user to avoid a segfault. Signed-off-by: Clemens Fruhwirth Signed-off-by: Junio C Hamano --- builtin/fetch.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/builtin/fetch.c b/builtin/fetch.c index dfde96a435..0a11430df4 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -1602,6 +1602,10 @@ static int do_fetch(struct transport *transport, struct ref *rm; struct ref *source_ref = NULL; + if (!branch) { + warning(_("no branch detected to use --set-upstream with.")); + goto skip; + } /* * We're setting the upstream configuration for the * current branch. The relevant upstream is the -- 2.32.0-288-g654beb0eda ``` ## Junio C Hamano, 2021-07-06 19:50 Subject: Re: [PATCH] fetch: Fix segfault on pull --set-upstream outside a branch. Message-ID: URL: https://gitlist.dev/e/xmqqsg0ri5mq.fsf%40gitster.g In-Reply-To: ``` Junio C Hamano writes: > Junio C Hamano writes: > >> Nit. What do you mean by "outside a branch"? If you mean "when the >> HEAD is detached", please say so, and use the same terminology in >> your warning message. > > Here is what I queued for today's integration run. Eric's > suggestion to add a simple test is a good idea, too. Oops, wrong patch. -- >8 -- From: Clemens Fruhwirth Date: Tue, 6 Jul 2021 18:22:38 +0200 Subject: [PATCH] fetch: fix segfault on --set-upstream while on a detached HEAD branch_get("HEAD") can return NULL, when HEAD is detached, and cause the code to segfault when the user calls "git pull --set-upstream". Catch this case and warn the user to avoid a segfault. Signed-off-by: Clemens Fruhwirth Signed-off-by: Junio C Hamano --- builtin/fetch.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/builtin/fetch.c b/builtin/fetch.c index dfde96a435..4efdd28396 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -1602,6 +1602,10 @@ static int do_fetch(struct transport *transport, struct ref *rm; struct ref *source_ref = NULL; + if (!branch) { + warning(_("not on a branch to use --set-upstream with")); + goto skip; + } /* * We're setting the upstream configuration for the * current branch. The relevant upstream is the -- 2.32.0-288-g654beb0eda ```