Paul - You are correct :) I've played with the idea of switching over to an enum, the one issue I've run into though that sort of stayed my hand was the AFI_IP -vs- AF_INET dichotomy that sometimes get's into play and causes issues.
donald On Tue, Dec 15, 2015 at 11:22 AM, Paul Jakma <[email protected]> wrote: > On Tue, 15 Dec 2015, Donald Sharp wrote: > > Martin - >> >> As discussed in the meeting today here is an example of a bogus issue: >> >> >> https://ci1.netdef.org/artifact/QUAGGA-QMASTER/shared/build-82/static_analysis/report-8a9e85.html#EndPath >> >> Basically the error message boils down to this code construct: >> >> struct prefix p; >> if (afi == v4) >> set p appropriately >> else if (afi == v6) >> set p appropriately >> >> if (p.prefixlen) <------- If afi is not v4 or v6 then prefix length is >> not going to be set. >> >> I spent a few minutes looking at clang and saw no good way to mark the >> code >> in plist.c as ok. >> > > To be fair, the compiler is technically correct - the best kind of > correct. :) > > It'd be nice if setting afi_t to an enum with just those 2 values was > sufficient, but it seems clang still does: > > if (afi == v4) > /* take false */ > if (afi == v6) > /* take false */ > > However, enum + a switch with just those 2 values and clang shuts up. See > below. > > A more compact diff would just be: > > if (!(afi == AFI_IP || afi == AFI_IP6)) > return CMD_WARNING; > > at the top of that func, but the enum could help compilers. > > diff --git a/lib/plist.c b/lib/plist.c > index f9e626d..6e00869 100644 > --- a/lib/plist.c > +++ b/lib/plist.c > @@ -698,44 +698,45 @@ vty_prefix_list_install (struct vty *vty, afi_t afi, > const char *name, > } > > /* "any" is special token for matching any IPv4 addresses. */ > - if (afi == AFI_IP) > - { > - if (strncmp ("any", prefix, strlen (prefix)) == 0) > - { > - ret = str2prefix_ipv4 ("0.0.0.0/0", (struct prefix_ipv4 *) &p); > - genum = 0; > - lenum = IPV4_MAX_BITLEN; > - any = 1; > - } > - else > - ret = str2prefix_ipv4 (prefix, (struct prefix_ipv4 *) &p); > - > - if (ret <= 0) > - { > - vty_out (vty, "%% Malformed IPv4 prefix%s", VTY_NEWLINE); > - return CMD_WARNING; > - } > - } > -#ifdef HAVE_IPV6 > - else if (afi == AFI_IP6) > + switch (afi) > { > - if (strncmp ("any", prefix, strlen (prefix)) == 0) > - { > - ret = str2prefix_ipv6 ("::/0", (struct prefix_ipv6 *) &p); > - genum = 0; > - lenum = IPV6_MAX_BITLEN; > - any = 1; > - } > - else > - ret = str2prefix_ipv6 (prefix, (struct prefix_ipv6 *) &p); > - > - if (ret <= 0) > - { > - vty_out (vty, "%% Malformed IPv6 prefix%s", VTY_NEWLINE); > - return CMD_WARNING; > - } > + case AFI_IP: > + { > + if (strncmp ("any", prefix, strlen (prefix)) == 0) > + { > + ret = str2prefix_ipv4 ("0.0.0.0/0", (struct prefix_ipv4 *) > &p); > + genum = 0; > + lenum = IPV4_MAX_BITLEN; > + any = 1; > + } > + else > + ret = str2prefix_ipv4 (prefix, (struct prefix_ipv4 *) &p); > + > + if (ret <= 0) > + { > + vty_out (vty, "%% Malformed IPv4 prefix%s", VTY_NEWLINE); > + return CMD_WARNING; > + } > + } break; > + case AFI_IP6: > + { > + if (strncmp ("any", prefix, strlen (prefix)) == 0) > + { > + ret = str2prefix_ipv6 ("::/0", (struct prefix_ipv6 *) &p); > + genum = 0; > + lenum = IPV6_MAX_BITLEN; > + any = 1; > + } > + else > + ret = str2prefix_ipv6 (prefix, (struct prefix_ipv6 *) &p); > + > + if (ret <= 0) > + { > + vty_out (vty, "%% Malformed IPv6 prefix%s", VTY_NEWLINE); > + return CMD_WARNING; > + } > + } break; > } > -#endif /* HAVE_IPV6 */ > > /* ge and le check. */ > if (genum && (genum <= p.prefixlen)) > diff --git a/lib/zebra.h b/lib/zebra.h > index a607437..97ed05c 100644 > --- a/lib/zebra.h > +++ b/lib/zebra.h > @@ -483,15 +483,19 @@ extern const char *zserv_command_string (unsigned > int command); > #endif > > /* Address family numbers from RFC1700. */ > -#define AFI_IP 1 > -#define AFI_IP6 2 > +typedef enum { > + AFI_IP = 1, > + AFI_IP6 = 2, > +} afi_t; > #define AFI_MAX 3 > > /* Subsequent Address Family Identifier. */ > -#define SAFI_UNICAST 1 > -#define SAFI_MULTICAST 2 > -#define SAFI_RESERVED_3 3 > -#define SAFI_MPLS_VPN 4 > +typedef enum { > + SAFI_UNICAST = 1, > + SAFI_MULTICAST = 2, > + SAFI_RESERVED_3 = 3, > + SAFI_MPLS_VPN = 4, > +} safi_t; > #define SAFI_MAX 5 > > /* Filter direction. */ > @@ -516,10 +520,6 @@ extern const char *zserv_command_string (unsigned int > command); > #define SET_FLAG(V,F) (V) |= (F) > #define UNSET_FLAG(V,F) (V) &= ~(F) > > -/* AFI and SAFI type. */ > -typedef u_int16_t afi_t; > -typedef u_int8_t safi_t; > - > /* Zebra types. Used in Zserv message header. */ > typedef u_int16_t zebra_size_t; > typedef u_int16_t zebra_command_t; > > regards, > -- > Paul Jakma [email protected] @pjakma Key ID: 64A2FF6A > Fortune: > Going the speed of light is bad for your age. >
_______________________________________________ Quagga-dev mailing list [email protected] https://lists.quagga.net/mailman/listinfo/quagga-dev
