Re: [PATCH] branch_get_push: do not segfault when HEAD is detached

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

Hi Kyle,

On Thu, 5 Jan 2017, Kyle Meyer wrote:

> Move the detached HEAD check from branch_get_push_1() to
> branch_get_push() to avoid setting branch->push_tracking_ref when
> branch is NULL.
> 
> Signed-off-by: Kyle Meyer <kyle@kyleam.com>

Good point.

> diff --git a/remote.c b/remote.c
> index ad6c5424e..d5eaec737 100644
> --- a/remote.c
> +++ b/remote.c
> @@ -1716,9 +1716,6 @@ static const char *branch_get_push_1(struct branch *branch, struct strbuf *err)
>  {
>  	struct remote *remote;
>  
> -	if (!branch)
> -		return error_buf(err, _("HEAD does not point to a branch"));
> -
>  	remote = remote_get(pushremote_for_branch(branch, NULL));
>  	if (!remote)
>  		return error_buf(err,
> @@ -1778,6 +1775,9 @@ static const char *branch_get_push_1(struct branch *branch, struct strbuf *err)
>  
>  const char *branch_get_push(struct branch *branch, struct strbuf *err)
>  {
> +	if (!branch)
> +		return error_buf(err, _("HEAD does not point to a branch"));
> +
>  	if (!branch->push_tracking_ref)
>  		branch->push_tracking_ref = branch_get_push_1(branch, err);

This is the only caller of branch_get_push_1(), so all is good.

> diff --git a/t/t1514-rev-parse-push.sh b/t/t1514-rev-parse-push.sh
> index 7214f5b33..90c639ae1 100755
> --- a/t/t1514-rev-parse-push.sh
> +++ b/t/t1514-rev-parse-push.sh
> @@ -60,4 +60,10 @@ test_expect_success '@{push} with push refspecs' '
>  	resolve topic@{push} refs/remotes/origin/magic/topic
>  '
>  
> +test_expect_success 'resolving @{push} fails with a detached HEAD' '
> +	git checkout HEAD^{} &&

I seem to recall that we prefer HEAD^0 over HEAD^{} and the existing
scripts seem to agree with me:

$ git grep -c HEAD^0 junio/pu -- t/
junio/pu:t/t1450-fsck.sh:1
junio/pu:t/t1507-rev-parse-upstream.sh:2
junio/pu:t/t2020-checkout-detach.sh:5
junio/pu:t/t3200-branch.sh:1
junio/pu:t/t3203-branch-output.sh:3
junio/pu:t/t3400-rebase.sh:1
junio/pu:t/t3404-rebase-interactive.sh:1
junio/pu:t/t5407-post-rewrite-hook.sh:2
junio/pu:t/t5505-remote.sh:1
junio/pu:t/t5510-fetch.sh:1
junio/pu:t/t5533-push-cas.sh:3
junio/pu:t/t6035-merge-dir-to-symlink.sh:3
junio/pu:t/t7201-co.sh:2
junio/pu:t/t7402-submodule-rebase.sh:1
junio/pu:t/t9105-git-svn-commit-diff.sh:1
junio/pu:t/t9107-git-svn-migrate.sh:1

$ git grep -c HEAD^{} junio/pu -- t/
junio/pu:t/t3200-branch.sh:3

Maybe use HEAD^0 just for consistency?

Ciao,
Johannes