[Swan-dev] ROOF changes in commit 91a1e8537
D. Hugh Redelmeier
hugh at mimosa.com
Mon Mar 13 14:22:56 UTC 2017
| From: Paul Wouters <paul at nohats.ca>
| IKEv2_ENCR_CAMELLIA_CCM_C = 27, /* CAMELLIA_CCM_16 RFC 5529 */
| IKEv2_ENCR_CHACHA20_POLY1305 = 28, /* RFC7634 */
| - IKEv2_ENCR_ROOF = IKEv2_ENCR_CHACHA20_POLY1305,
| + IKEv2_ENCR_ROOF,
| I'm a bit concerned about you adding an algorithm entry in the list that
| is not real. What happens now when we receive an ENCR algo with number 29 ?
Nothing in the code that validates an algo number is affected by adding
another enum. (I don't know how it is checked but don't need to to make
that claim: this claim is based on the C language.)
| I'm surprised this passed the internal enum check too without crashing
| because no name was added to the enum_names in constants.c ?
I certainly didn't add a enum name. That would have created problems.
| Maybe I should have called these MAX instead of ROOF? I purposefully
| avoided adding an entry and choose to just have an alias name instead.
Yes, if you mean "largest value", "MAX" is a better term. ROOF is the
convention in Pluto for "largest + 1".
In C, ROOF is very often more convenient. Look at the diff: a bunch
of characters were eliminated. All the " + 1" expressions are a bit
tiresome and easy to get wrong.
Even better, the ROOF definitions are implicitly given the right
number by the C compiler. This makes adding new entries easier and
less error prone. Mind you, it requires that the largest entry must
be defined immediately previously.
Note: one problem with ROOF is that in certain conditions it may not
be representable in the type. Consider a case where the maximum value
is INT_MAX. INT_ROOF cannot be represented as an int. That doesn't
apply with the examples in Pluto.
Another slight annoyance: some C compilers will warn you about switch
statements that don't have a case for each enum value. Clearly you don't
want a "case ..._ROOF:". Adding a "default:" would shut it up. This did
not come up with the commit.
More information about the Swan-dev