Yes, bug_on(x) is good point, so I am expecting you will continue review the rest patches ?
Thanks, On Tue, Aug 2, 2016 at 6:50 PM, Tobias Klauser <[email protected]> wrote: > On 2016-07-26 at 21:35:09 +0200, Vadim Kochan <[email protected]> wrote: >> Usually proto fields array is sorted in the same order as related enum, >> so id may be used as index for faster lookup, it will make >> csum field calculation little faster at runtime. >> >> Signed-off-by: Vadim Kochan <[email protected]> >> --- >> trafgen_l3.h | 4 ++-- >> trafgen_proto.c | 8 +------- >> 2 files changed, 3 insertions(+), 9 deletions(-) >> >> diff --git a/trafgen_l3.h b/trafgen_l3.h >> index a1b1523..e0c9a1c 100644 >> --- a/trafgen_l3.h >> +++ b/trafgen_l3.h >> @@ -10,14 +10,14 @@ enum ip4_field { >> IP4_LEN, >> IP4_ID, >> IP4_FLAGS, >> + IP4_MF, >> + IP4_DF, >> IP4_FRAG_OFFS, >> IP4_TTL, >> IP4_PROTO, >> IP4_CSUM, >> IP4_SADDR, >> IP4_DADDR, >> - IP4_DF, >> - IP4_MF, >> }; >> >> enum ip6_field { >> diff --git a/trafgen_proto.c b/trafgen_proto.c >> index a1d56cf..ce389ce 100644 >> --- a/trafgen_proto.c >> +++ b/trafgen_proto.c >> @@ -118,13 +118,7 @@ void proto_header_fields_add(struct proto_hdr *hdr, >> >> static struct proto_field *proto_field_by_id(struct proto_hdr *hdr, >> uint32_t fid) >> { >> - int i; >> - >> - for (i = 0; i < hdr->fields_count; i++) >> - if (hdr->fields[i].id == fid) >> - return &hdr->fields[i]; >> - >> - panic("Failed lookup field id %u for proto id %u\n", fid, hdr->id); >> + return &hdr->fields[fid]; > > Please add an inline comment here explaining this behaviour. Otherwise > someone will stumble upon it in the future for sure. > > Also, something along the lines of > > bug_on(hdr->fields[fid].id != fid); > > should be added to catch future erroneous fields definitions. -- You received this message because you are subscribed to the Google Groups "netsniff-ng" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. For more options, visit https://groups.google.com/d/optout.
