> On 7 May 2017, at 20:15, Steffan Karger <stef...@karger.me> wrote:
>
> Hi,
>
> On 07-05-17 14:03, Antonio Quartulli wrote:
>>
>>> On 7 May 2017, at 19:56, Antonio Quartulli <a...@unstable.cc> wrote:
>>>
>>>> On 4 May 2017, at 06:57, David Sommerseth
>>>> <open...@sf.lists.topphemmelig.net> wrote:
>>>>
>>>> On 03/05/17 22:15, Steffan Karger wrote:
>>>>>> + switch (opt->verify_hash_algo)
>>>>>> + {
>>>>>> + case MD_SHA1:
>>>>>> + ca_hash = x509_get_sha1_fingerprint(cert, &gc);
>>>>>> + break;
>>>>>> +
>>>>>> + case MD_SHA256:
>>>>>> + ca_hash = x509_get_sha256_fingerprint(cert, &gc);
>>>>>> + break;
>>>>>> + }
>>>>> This switch should have a default: case that fails, otherwise we might
>>>>> be reading from uninitialized memory. And you might want to consider
>>>>> initializing ca_hash to "{ 0 }". (But you still need to default: case,
>>>>> otherwise you'll be doing a 0-length memcmp()).
>>>>
>>>> *grmbl* I was sure the enumerated type would make the compiler
>>>> complain, but I must have mixed this with some C++ stuff I read not too
>>>> long ago. But my stupid test allows assigning an enumerated type to any
>>>> invalid value as well.
>>>>
>>>
>>> You are right.
>>> The compiler would have complained if you had left out any value from the
>>> enum set.
>>> But the enum set is made of those two values only, hence a default case is
>>> not plausible unless you
>>> assign verify_hash_algo with a value out of the enum set (but then you
>>> should have a compiler warning
>>> on the assignment).
>>>
>>
>> I forgot: the good point of *not* having a default is that when you will add
>> a new enum value for that
>> particular enum type, the compiler will throw warnings for every switch
>> block you have, informing you
>> that now they need to be updated. Having a default will leave you with
>> manually check the various
>> code spots.
>
> This only holds when compiling with -Wall, which we do not do by default
> (because our current code would emit quite some warnings).
>
> If we would fix our code and compile with -Wall -Werror, I'd agree. But
> as long as we don't, I prefer a defensive code style where I can exclude
> undefined behaviour just by looking at the code.
> Agreed. This sounds like a call for a cleanup mission :] Cheers, -- Antonio Quartulli
signature.asc
Description: Message signed with OpenPGP
------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
