[Swan-dev] ROOF changes in commit 91a1e8537

[D. Hugh Redelmeier]

| 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.