From: Dave Borowitz <dborowitz@google.com>
Date: Wed, 19 Aug 2015 15:18:02 GMT
Subject: Re: [PATCH 6/7] Support signing pushes iff the server supports it
Message-ID: <CAD0k6qRPmJAF4-LT1jZTCjTiGMpfS4+gt2C36izt5qe3P1pJig@mail.gmail.com>
In-Reply-To: <xmqqio8hfobk.fsf@gitster.dls.corp.google.com>

On Fri, Aug 14, 2015 at 7:22 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Dave Borowitz <dborowitz@google.com> 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...).

Found a slightly less repetitious way.