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

[Toke Høiland-Jørgensen]

Ondrej Zajicek <santiago at crfreenet.org> writes:

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

Heh, fair point :)

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

Yeah, got confused by that myself once or twice; I'll see if I can't
come up with something better.

-Toke