[PATCH v3 6/7] babel: Refactor TLV parsing code for easier reuse

[Ondrej Zajicek]

On Tue, Nov 24, 2020 at 04:21:53PM +0100, Toke Høiland-Jørgensen wrote:
> +/* Helper macros to loop over a series of TLVs.
> + * @start pointer to first TLV
> + * @end   byte * pointer to TLV stream end
> + * @tlv   struct babel_tlv pointer used as iterator
> + */
> +#define WALK_TLVS(start, end, tlv, saddr, ifname)                       \
> +  for (tlv = (void *)start;						\
> +       (byte *)tlv < end;						\
> +       tlv = NEXT_TLV(tlv))						\
> +  {									\
> +    byte *loop_pos;							\
> +    /* Ugly special case */						\
> +    if (tlv->type == BABEL_TLV_PAD1)					\
> +      continue;                                                         \
> +									\
> +    /* The end of the common TLV header */				\
> +    loop_pos = (byte *)tlv + sizeof(struct babel_tlv);			\
> +    if ((loop_pos > end) || (loop_pos + tlv->length > end))             \

...

>  static inline int
>  babel_read_subtlvs(struct babel_tlv *hdr,
>  		   union babel_msg *msg,
>  		   struct babel_parse_state *state)
>  {
> +  const struct babel_tlv_data *tlv_data;
> +  struct babel_proto *p = state->proto;
>    struct babel_tlv *tlv;
> -  byte *pos, *end = (byte *) hdr + TLV_LENGTH(hdr);
> +  byte *end = (byte *) hdr + TLV_LENGTH(hdr);
>    int res;
>  
> -  for (tlv = (void *) hdr + state->current_tlv_endpos;
> -       (byte *) tlv < end;
> -       tlv = NEXT_TLV(tlv))
> +  WALK_TLVS(hdr + state->current_tlv_endpos, end, tlv,
> +            state->saddr, state->ifa->ifname)
>    {

Huh? This macro usage depends on typecast of a part if its first argument.
That is completely confusing. One see 'hdr + state->current_tlv_endpos',
but it is a different operation.

Also jumping from such macro to a fixed label is a bit confusing ...

-- 
Elen sila lumenn' omentielvo

Ondrej 'Santiago' Zajicek (email: santiago at crfreenet.org)
OpenPGP encrypted e-mails preferred (KeyID 0x11DEADC3, wwwkeys.pgp.net)
"To err is human -- to blame it on a computer is even more so."