Hi, On 05/12/17 02:45, Gert Doering wrote: > Hi, > > On Sun, Dec 03, 2017 at 03:17:51PM +0100, Steffan Karger wrote: >> On 02-12-17 14:45, Antonio Quartulli wrote: >>> Now that ENABLE_CRYPTO has been removed, CIPHER_ENABLED is basically >>> a useless shortcut which does not really help the readability of the >>> code. > > Call me silly, but CIPHER_ENABLED has nothing to do with ENABLE_CRYPTO, > but more with "--cipher none", no? >
"Remove ENABLE_CRYPTO" introduced this change: -#ifdef ENABLE_CRYPTO #define CIPHER_ENABLED(c) (c->c1.ks.key_type.cipher != NULL) -#else -#define CIPHER_ENABLED(c) (false) -#endif thus my feeling was that this macro is now not so useful, because there is no conditional definition anymore. > So at least the commit message is misleading... > >>> * Set adjustment factor for buffer alignment when no >>> * cipher is used. >>> */ >>> - if (!CIPHER_ENABLED(c)) >>> + if (!c->c1.ks.key_type.cipher) > > ... and I don't really find the second one more easily understandable > than the first one... > I think it's a matter of taste :-) We could substitute every NULL check with a human readable macro/inline-functions, but I am not sure it would be really useful. Honestly, this condition seem quite clear to me (at least compared to the TLS_MODE()/tls_multi one). But, again, this is just my taste: feel free to drop this patch if you think it is better to keep the macro. Cheers, -- Antonio Quartulli
signature.asc
Description: OpenPGP digital signature
------------------------------------------------------------------------------ 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
