[ovs-dev] [ofp-print 16/18] Make compiler complain about unhandled OpenFlow and Nicira action types.
Ben Pfaff
blp at nicira.com
Thu Dec 9 18:40:25 UTC 2010
On Wed, Dec 08, 2010 at 11:21:16PM -0800, Justin Pettit wrote:
> On Dec 8, 2010, at 4:27 PM, Ben Pfaff wrote:
>
> > static void
> > ofp_print_nx_action(struct ds *string, const struct nx_action_header *nah)
> > {
> > + enum nx_action_subtype subtype = ntohs(nah->subtype);
> > + switch (subtype) {
> > + case NXAST_SNAT__OBSOLETE:
> > + break;
>
> Do you think we should print a warning for obsoleted actions?
Might as well, I just moved this to the "default" case.
> > default:
> > - ds_put_format(string, "***unknown Nicira action:%d***",
> > + ds_put_format(string, "***unknown Nicira action:%"PRIu16"***",
> > ntohs(nah->subtype));
>
> You can just use "subtype".
No, that might have been truncated to the range of enum
nx_action_subtype.
> > @@ -1680,6 +1681,7 @@ check_nicira_action(const union ofp_action *a, unsigned int len,
> > const struct flow *flow)
> > {
> > const struct nx_action_header *nah;
> > + uint16_t subtype;
>
> What's the reason for not treating this as a enum "nx_action_subtype"
> immediately?
>
> > + subtype = ntohs(nah->subtype);
> > + if (subtype > TYPE_MAXIMUM(enum nx_action_subtype)) {
> > + /* This is necessary because enum nx_action_subtype is probably an
> > + * 8-bit type, so the cast below throws away the top 8 bits. */
> > + return ofp_mkerr(OFPET_BAD_ACTION, OFPBAC_BAD_VENDOR_TYPE);
> > + }
>
> Why is nx_action_subtype likely a 8-bit type?
When you define an enumeration, the compiler gets to pick the integer
type to represent it. It has to pick a type that contains all the
values, and normally it picks the smallest type that fits. The smallest
value in this enumeration is 0, and the largest is 7, so it probably
picks uint8_t or int8_t.
More information about the dev
mailing list