Re: [PATCH v3] http: add support for specifying the SSL version

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



2015-08-13 18:11 GMT+02:00 Eric Sunshine <sunshine@xxxxxxxxxxxxxx>:
> On Thu, Aug 13, 2015 at 11:58 AM, Elia Pinto <gitter.spiros@xxxxxxxxx> wrote:
>> 2015-08-13 17:47 GMT+02:00 Eric Sunshine <sunshine@xxxxxxxxxxxxxx>:
>>> On Thu, Aug 13, 2015 at 11:28 AM, Elia Pinto <gitter.spiros@xxxxxxxxx> wrote:
>>>> Teach git about a new option, "http.sslVersion", which permits one to
>>>> specify the SSL version  to use when negotiating SSL connections.  The
>>>> setting can be overridden by the GIT_SSL_VERSION environment
>>>> variable.
>>>>
>>>> Signed-off-by: Elia Pinto <gitter.spiros@xxxxxxxxx>
>>>> ---
>>>> This is the third version of the patch. The changes compared to the previous version are:
>>>
>>> Looks better. A few comments below...
>>>
>>>> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
>>>> index c97c648..6e9359c 100644
>>>> --- a/contrib/completion/git-completion.bash
>>>> +++ b/contrib/completion/git-completion.bash
>>>> @@ -364,9 +381,22 @@ static CURL *get_curl_handle(void)
>>>>         if (http_proactive_auth)
>>>>                 init_curl_http_auth(result);
>>>>
>>>> +       if (getenv("GIT_SSL_VERSION"))
>>>> +               ssl_version = getenv("GIT_SSL_VERSION");
>>>> +       if (ssl_version != NULL && *ssl_version) {
>>>> +               int i;
>>>> +               for ( i = 0; i < ARRAY_SIZE(sslversions); i++ ) {
>>>> +                       if (sslversions[i].name != NULL && *sslversions[i].name && !strcmp(ssl_version,sslversions[i].name)) {
>>>
>>> This sort of loop is normally either handled by indexing up to a limit
>>> (ARRAY_SIZE, in this case) or by iterating until hitting a sentinel
>>> (NULL, in this case). It is redundant to use both, as this code does.
>> I do not think. sslversions[i].name can be null, see how the structure
>> is initialized. No ?
>
> The initialization:
>
>     static struct {
>        const char *name;
>        long ssl_version;
>        } sslversions[] = {
>            { "sslv2", CURL_SSLVERSION_SSLv2 },
>            ...
>            { "tlsv1.2", CURL_SSLVERSION_TLSv1_2 },
>            { NULL }
>     };
>
> terminates the list with a NULL sentinel entry, which does indeed set
> sslversions[i].name to NULL. When you know the item count ahead of
> time (as you do in this case), this sort of end-of-list sentinel is
> redundant, and complicates the code unnecessarily. For instance, the
> 'sslversions[i].name != NULL' expression in the 'if':
>
>     if (sslversions[i].name != NULL && *sslversions[i].name ...
>
> is an unwanted complication. In fact, the '*sslversions[i].name'
> expression is also unnecessary.
I agree. But this is what  suggested me Junio: =). What do I have to do ?
It becomes difficult to keep everyone happy: =)

Junio ?

Thanks
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]