On Mon, Jan 15, 2018 at 5:33 PM, Emmanuel Deloget <log...@free.fr> wrote:
> Hello Steffan,
>
> On Sun, Jan 14, 2018 at 11:26 AM, Steffan Karger <stef...@karger.me>
> wrote:
>
>> Hi,
>>
>> On 12-01-18 22:37, Emmanuel Deloget wrote:
>> > Calling EVP_KEY_id() before EVP_PKEY_get0_*() is unnecessary as
>> > the same check is also performed in the later.
>> >
>> > We also make the code a bit better by not calling the various
>> > EVP_PKEY_get0_*() functions twice (this needs a bit or reordering to
>> > avoid introducing yet another #ifndef OPENSSL_NO_EC in the code).
>>
>> Checking double is a bit silly, but we can fix that by simply removing
>> the "EVP_PKEY_id() == ... && " occurrences. (That still allows us to
>> remove it from the compat wrapper.)
>>
>> I'm not sure that moving the variables outside the if() scope actually
>> improves the code. At least I think the original flow is easier to
>> read. Mostly due to the #ifdef noise, but still. In a path that's not
>> performance-critical, I prefer readability over performance.
>>
>
>
> Well, to be honest, I was definitely not trying to make any kind of
> performance gain :)
>
> The whole idea was to make code easier to read and to remove some now
> weird duplication of functions (the kind of duplication that will come and
> bites you later).
>
> For the variables outside the ifs, the next C standard should allow us to
> write something like:
>
> if ((RSA *rsa = EVP_PKEY_get0_RSA(pkey)) != NULL) {
> ...
>
> That should be easier to read (but then, I'm not sure when this will be
> considered as widespread ; I guess we're slated for 2025 or so :))
>
> Anyway, moving the variables within the if means we're going to call
> EVP_PKEY_get0_RSA() twice which, I feel, is not a good thing to do. But
> even if I'm not very comfortable with this, I can still propose a patch
> that des it (it's lighter than the currently proposed change). You tell me
> :)
>
Can I try one last time to convince you guys to keep EVP_PKEY_id()? If I
was targeting only 1.1.0+ that's how most of those checks would have been
done, isn't? Makes life easier, code simpler as I have said before..
Just throwing in my 0.02c, feel free to ignore.
Best,
Selva
------------------------------------------------------------------------------
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
