[PATCH 33/43] site.c, magic.h: Formalize the system of message variants.

[Ian Jackson]

Mark Wooding writes ("[PATCH 33/43] site.c, magic.h: Formalize the system of message variants."):
> Introduce macros for splitting message-type labels into a `base' code
> and `variant' number.  Use these for dealing with the two existing
> `MSG3' variants, which will make it easy to deal with more in the
> future.  Hint hint.

I apologise for contributing to the awkward bit patterns here.

> +#define MSGTYPE_BASE(type) ((type)&0x0fffff0f)
> +#define MSGTYPE_VAR(type) ((type)&0xf00000f0)
> +
> +#define MSGVAR(i)     (((i) << 4) | ((i) << 28))
> +#define LABEL_MSG3BIS (LABEL_MSG3 | MSGVAR(1))

This leaves us with only 4 bits of subtype.  I wonder if

 +#define MSGVAR(i)     (((i & 0xf) << 4) ^ ((i & 0xf) << 28) ^ 
                         ((i & 0xf0) << 24))

would be better.

But:

> -	type == LABEL_MSG3 ||
> -	type == LABEL_MSG3BIS ||
> -	type == LABEL_MSG4;
> +	MSGTYPE_BASE(type) == LABEL_MSG3 ||
> +	MSGTYPE_BASE(type) == LABEL_MSG4;
...
> -    if (type==LABEL_MSG3BIS)
> +    if (MSGTYPE_BASE(type) == LABEL_MSG3) do {
...
> -    if (type==LABEL_MSG3BIS) {
> +    if (MSGTYPE_BASE(type) == LABEL_MSG3) do {

The trouble with this approach is that it changes the handling of
unknown message variants, from "reject as unknown message type" to
"mishandle as if we understood it".

Of course such things shouldn't be sent.  But I think they should be
detected and rejcted.


The alternative approach, of simply introducing MSG3TER (or, perhaps,
MSG3v2), seems simpler.  There are 4 places where _all_ of the MSG3
variants need to be named: type_is_msg34; check_msg; process_msg3;
site_incoming's big switch.  These could all be handled with

  /* use like this: `case LABEL_MSGv3ANY_CASE:' */
  #define MSG3vANY_CASE   \
           LABEL_MSG3:    \
      case LABEL_MSG3v1:  \
      case LABEL_MSG3v2

or something.

Ian.