Subject: Re: [PATCH 6/7] Support signing pushes iff the server supports it

From: Junio C Hamano <[email-masked]>
Date: Fri, 14 Aug 2015 16:22:07 -0700

Dave Borowitz <[email-masked]> writes:

> diff --git a/send-pack.c b/send-pack.c
> index 2a64fec..6ae9f45 100644
> --- a/send-pack.c
> +++ b/send-pack.c
> @@ -370,7 +370,7 @@ int send_pack(struct send_pack_args *args,
>  		args->use_thin_pack = 0;
>  	if (server_supports("atomic"))
>  		atomic_supported = 1;
> -	if (args->push_cert) {
> +	if (args->push_cert == SEND_PACK_PUSH_CERT_ALWAYS) {
>  		int len;
>  
>  		push_cert_nonce = server_feature_value("push-cert", &len);
> @@ -379,6 +379,18 @@ int send_pack(struct send_pack_args *args,
>  		reject_invalid_nonce(push_cert_nonce, len);
>  		push_cert_nonce = xmemdupz(push_cert_nonce, len);
>  	}
> +	if (args->push_cert == SEND_PACK_PUSH_CERT_IF_POSSIBLE) {
> +		int len;
> +
> +		push_cert_nonce = server_feature_value("push-cert", &len);
> +		if (push_cert_nonce) {
> +			reject_invalid_nonce(push_cert_nonce, len);
> +			push_cert_nonce = xmemdupz(push_cert_nonce, len);
> +		} else
> +			warning(_("not sending a push certificate since the"
> +				  " receiving end does not support --signed"
> +				  " push"));
> +	}

I wonder if the bodies of these two if statements can be a bit
better organized to avoid duplication (I suspect you have tried
and you may already know that the above is the most readable
version, but I haven't tried to do so myself, so...).