[Swan-dev] COOKIE_SIZE is IKEv1!

[Antony Antony]

On Mon, Jul 16, 2018 at 12:06:21PM -0400, Andrew Cagney wrote:
..

> > | and by using magic macros we've just burried what should be simple numbers.
> >
> > This I completely disagree with.  8 means many things.  COOKIE_SIZE
> > shows what the heck the number is.  And the name helps you find
> > related uses.  Good luck grepping for 8.
> 
> I'll take that as a rhetorical comment.  Code should either be using
> sizeof, or more likely spi_size - since it can change from the POV of
> common code.  Our cookie hashing code cheats and hashes zeros.
> 
> > We do not drop magic numbers into our code.  I'm surprised you think
> > that we should.
> 
> I'm talking about defining macros for the macros sake.  Consider:
> 
> const struct encrypt_desc ike_alg_encrypt_aes_cbc = {
> ...
>         .keydeflen =  128,
>         .key_bit_lengths = { 256, 192, 128, },
> ...
> 

though this may appear clearer as its own, it would get messy in real life.

say later you want to equate this some where else,
How would this style extend ? 

e.g

if (ike_enc.len == 128 )   {
}

I gues for this you would need AES_KEY_DEF_LEN
If you have that, then the above style is confusing.
Once you have AES_KEY_DEF_LEN  defined for ohter reasons stick to it.
Do not mix it.

> vs:
> 
> const struct encrypt_desc ike_alg_encrypt_aes_cbc = {
> ...
>         .keydeflen =    AES_KEY_DEF_LEN,
>         .key_bit_lengths = { AES_KEY_MAX_LEN, 192, AES_KEY_MIN_LEN, },
>  ...

this case would be clearer, if you also need it later on.

if (ike_enc.len == AES_KEY_DEF_LEN) {
}

> (neither case appears in the code).  Which is clearer?

most cases the second one.  However, there could very few situations where the first one is clearer.

-antony