From 0a2869d524fc3612b4b77a9df4991bff2aa3b465 Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Thu, 18 Feb 2016 15:13:09 -0800 Subject: [PATCH] ofpbuf: New function ofpbuf_const_initializer(). A number of times I've looked at code and thought that it would be easier to understand if I could write an initializer instead of ofpbuf_use_const(). This commit adds a function for that purpose and adapts a lot of code to use it, in the places where I thought it made the code better. In theory this could improve code generation since the new function can be inlined whereas ofpbuf_use_const() isn't. But I guess that's probably insignificant; the intent of this change is code readability. Signed-off-by: Ben Pfaff Acked-by: Jarno Rajahalme --- lib/dpif-netlink.c | 72 +++----- lib/learning-switch.c | 6 +- lib/netdev-windows.c | 18 +- lib/netlink-socket.c | 9 +- lib/nx-match.c | 15 +- lib/ofp-actions.c | 79 ++++----- lib/ofp-errors.c | 8 +- lib/ofp-msgs.c | 4 +- lib/ofp-print.c | 67 ++------ lib/ofp-util.c | 318 ++++++++++++----------------------- lib/ofpbuf.h | 26 ++- ofproto/ofproto-dpif-xlate.c | 8 +- ofproto/ofproto.c | 35 ++-- utilities/ovs-ofctl.c | 8 +- 14 files changed, 249 insertions(+), 424 deletions(-) diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c index 5e4839378..f1eaa5124 100644 --- a/lib/dpif-netlink.c +++ b/lib/dpif-netlink.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014, 2015 Nicira, Inc. + * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014, 2015, 2016 Nicira, Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -1972,18 +1972,12 @@ parse_odp_packet(const struct dpif_netlink *dpif, struct ofpbuf *buf, [OVS_PACKET_ATTR_MRU] = { .type = NL_A_U16, .optional = true } }; - struct ovs_header *ovs_header; - struct nlattr *a[ARRAY_SIZE(ovs_packet_policy)]; - struct nlmsghdr *nlmsg; - struct genlmsghdr *genl; - struct ofpbuf b; - int type; - - ofpbuf_use_const(&b, buf->data, buf->size); + struct ofpbuf b = ofpbuf_const_initializer(buf->data, buf->size); + struct nlmsghdr *nlmsg = ofpbuf_try_pull(&b, sizeof *nlmsg); + struct genlmsghdr *genl = ofpbuf_try_pull(&b, sizeof *genl); + struct ovs_header *ovs_header = ofpbuf_try_pull(&b, sizeof *ovs_header); - nlmsg = ofpbuf_try_pull(&b, sizeof *nlmsg); - genl = ofpbuf_try_pull(&b, sizeof *genl); - ovs_header = ofpbuf_try_pull(&b, sizeof *ovs_header); + struct nlattr *a[ARRAY_SIZE(ovs_packet_policy)]; if (!nlmsg || !genl || !ovs_header || nlmsg->nlmsg_type != ovs_packet_family || !nl_policy_parse(&b, 0, ovs_packet_policy, a, @@ -1991,9 +1985,9 @@ parse_odp_packet(const struct dpif_netlink *dpif, struct ofpbuf *buf, return EINVAL; } - type = (genl->cmd == OVS_PACKET_CMD_MISS ? DPIF_UC_MISS - : genl->cmd == OVS_PACKET_CMD_ACTION ? DPIF_UC_ACTION - : -1); + int type = (genl->cmd == OVS_PACKET_CMD_MISS ? DPIF_UC_MISS + : genl->cmd == OVS_PACKET_CMD_ACTION ? DPIF_UC_ACTION + : -1); if (type < 0) { return EINVAL; } @@ -2469,18 +2463,14 @@ dpif_netlink_vport_from_ofpbuf(struct dpif_netlink_vport *vport, [OVS_VPORT_ATTR_OPTIONS] = { .type = NL_A_NESTED, .optional = true }, }; - struct nlattr *a[ARRAY_SIZE(ovs_vport_policy)]; - struct ovs_header *ovs_header; - struct nlmsghdr *nlmsg; - struct genlmsghdr *genl; - struct ofpbuf b; - dpif_netlink_vport_init(vport); - ofpbuf_use_const(&b, buf->data, buf->size); - nlmsg = ofpbuf_try_pull(&b, sizeof *nlmsg); - genl = ofpbuf_try_pull(&b, sizeof *genl); - ovs_header = ofpbuf_try_pull(&b, sizeof *ovs_header); + struct ofpbuf b = ofpbuf_const_initializer(buf->data, buf->size); + struct nlmsghdr *nlmsg = ofpbuf_try_pull(&b, sizeof *nlmsg); + struct genlmsghdr *genl = ofpbuf_try_pull(&b, sizeof *genl); + struct ovs_header *ovs_header = ofpbuf_try_pull(&b, sizeof *ovs_header); + + struct nlattr *a[ARRAY_SIZE(ovs_vport_policy)]; if (!nlmsg || !genl || !ovs_header || nlmsg->nlmsg_type != ovs_vport_family || !nl_policy_parse(&b, 0, ovs_vport_policy, a, @@ -2637,18 +2627,14 @@ dpif_netlink_dp_from_ofpbuf(struct dpif_netlink_dp *dp, const struct ofpbuf *buf .optional = true }, }; - struct nlattr *a[ARRAY_SIZE(ovs_datapath_policy)]; - struct ovs_header *ovs_header; - struct nlmsghdr *nlmsg; - struct genlmsghdr *genl; - struct ofpbuf b; - dpif_netlink_dp_init(dp); - ofpbuf_use_const(&b, buf->data, buf->size); - nlmsg = ofpbuf_try_pull(&b, sizeof *nlmsg); - genl = ofpbuf_try_pull(&b, sizeof *genl); - ovs_header = ofpbuf_try_pull(&b, sizeof *ovs_header); + struct ofpbuf b = ofpbuf_const_initializer(buf->data, buf->size); + struct nlmsghdr *nlmsg = ofpbuf_try_pull(&b, sizeof *nlmsg); + struct genlmsghdr *genl = ofpbuf_try_pull(&b, sizeof *genl); + struct ovs_header *ovs_header = ofpbuf_try_pull(&b, sizeof *ovs_header); + + struct nlattr *a[ARRAY_SIZE(ovs_datapath_policy)]; if (!nlmsg || !genl || !ovs_header || nlmsg->nlmsg_type != ovs_datapath_family || !nl_policy_parse(&b, 0, ovs_datapath_policy, a, @@ -2795,18 +2781,14 @@ dpif_netlink_flow_from_ofpbuf(struct dpif_netlink_flow *flow, /* The kernel never uses OVS_FLOW_ATTR_UFID_FLAGS. */ }; - struct nlattr *a[ARRAY_SIZE(ovs_flow_policy)]; - struct ovs_header *ovs_header; - struct nlmsghdr *nlmsg; - struct genlmsghdr *genl; - struct ofpbuf b; - dpif_netlink_flow_init(flow); - ofpbuf_use_const(&b, buf->data, buf->size); - nlmsg = ofpbuf_try_pull(&b, sizeof *nlmsg); - genl = ofpbuf_try_pull(&b, sizeof *genl); - ovs_header = ofpbuf_try_pull(&b, sizeof *ovs_header); + struct ofpbuf b = ofpbuf_const_initializer(buf->data, buf->size); + struct nlmsghdr *nlmsg = ofpbuf_try_pull(&b, sizeof *nlmsg); + struct genlmsghdr *genl = ofpbuf_try_pull(&b, sizeof *genl); + struct ovs_header *ovs_header = ofpbuf_try_pull(&b, sizeof *ovs_header); + + struct nlattr *a[ARRAY_SIZE(ovs_flow_policy)]; if (!nlmsg || !genl || !ovs_header || nlmsg->nlmsg_type != ovs_flow_family || !nl_policy_parse(&b, 0, ovs_flow_policy, a, diff --git a/lib/learning-switch.c b/lib/learning-switch.c index 002b8181b..002710098 100644 --- a/lib/learning-switch.c +++ b/lib/learning-switch.c @@ -416,11 +416,9 @@ process_switch_features(struct lswitch *sw, struct ofp_header *oh) { struct ofputil_switch_features features; struct ofputil_phy_port port; - enum ofperr error; - struct ofpbuf b; - ofpbuf_use_const(&b, oh, ntohs(oh->length)); - error = ofputil_pull_switch_features(&b, &features); + struct ofpbuf b = ofpbuf_const_initializer(oh, ntohs(oh->length)); + enum ofperr error = ofputil_pull_switch_features(&b, &features); if (error) { VLOG_ERR("received invalid switch feature reply (%s)", ofperr_to_string(error)); diff --git a/lib/netdev-windows.c b/lib/netdev-windows.c index aef9d2ccd..093175f5a 100644 --- a/lib/netdev-windows.c +++ b/lib/netdev-windows.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2014 VMware, Inc. + * Copyright (c) 2014, 2016 VMware, Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -224,18 +224,14 @@ netdev_windows_netdev_from_ofpbuf(struct netdev_windows_netdev_info *info, [OVS_WIN_NETDEV_ATTR_IF_FLAGS] = { .type = NL_A_U32 }, }; - struct nlattr *a[ARRAY_SIZE(ovs_netdev_policy)]; - struct ovs_header *ovs_header; - struct nlmsghdr *nlmsg; - struct genlmsghdr *genl; - struct ofpbuf b; - netdev_windows_info_init(info); - ofpbuf_use_const(&b, buf->data, buf->size); - nlmsg = ofpbuf_try_pull(&b, sizeof *nlmsg); - genl = ofpbuf_try_pull(&b, sizeof *genl); - ovs_header = ofpbuf_try_pull(&b, sizeof *ovs_header); + struct ofpbuf b = ofpbuf_const_initializer(&b, buf->data, buf->size); + struct nlmsghdr *nlmsg = ofpbuf_try_pull(&b, sizeof *nlmsg); + struct genlmsghdr *genl = ofpbuf_try_pull(&b, sizeof *genl); + struct ovs_header *ovs_header = ofpbuf_try_pull(&b, sizeof *ovs_header); + + struct nlattr *a[ARRAY_SIZE(ovs_netdev_policy)]; if (!nlmsg || !genl || !ovs_header || nlmsg->nlmsg_type != ovs_win_netdev_family || !nl_policy_parse(&b, 0, ovs_netdev_policy, a, diff --git a/lib/netlink-socket.c b/lib/netlink-socket.c index 0cedb4159..bb45bb114 100644 --- a/lib/netlink-socket.c +++ b/lib/netlink-socket.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014 Nicira, Inc. + * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014, 2016 Nicira, Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -1801,15 +1801,12 @@ static void log_nlmsg(const char *function, int error, const void *message, size_t size, int protocol) { - struct ofpbuf buffer; - char *nlmsg; - if (!VLOG_IS_DBG_ENABLED()) { return; } - ofpbuf_use_const(&buffer, message, size); - nlmsg = nlmsg_to_string(&buffer, protocol); + struct ofpbuf buffer = ofpbuf_const_initializer(message, size); + char *nlmsg = nlmsg_to_string(&buffer, protocol); VLOG_DBG_RL(&rl, "%s (%s): %s", function, ovs_strerror(error), nlmsg); free(nlmsg); } diff --git a/lib/nx-match.c b/lib/nx-match.c index 0f695f042..4999b1ac9 100644 --- a/lib/nx-match.c +++ b/lib/nx-match.c @@ -475,8 +475,6 @@ static enum ofperr nx_pull_raw(const uint8_t *p, unsigned int match_len, bool strict, struct match *match, ovs_be64 *cookie, ovs_be64 *cookie_mask) { - struct ofpbuf b; - ovs_assert((cookie != NULL) == (cookie_mask != NULL)); match_init_catchall(match); @@ -484,7 +482,7 @@ nx_pull_raw(const uint8_t *p, unsigned int match_len, bool strict, *cookie = *cookie_mask = htonll(0); } - ofpbuf_use_const(&b, p, match_len); + struct ofpbuf b = ofpbuf_const_initializer(p, match_len); while (b.size) { const uint8_t *pos = b.data; const struct mf_field *field; @@ -648,9 +646,7 @@ enum ofperr oxm_pull_field_array(const void *fields_data, size_t fields_len, struct field_array *fa) { - struct ofpbuf b; - - ofpbuf_use_const(&b, fields_data, fields_len); + struct ofpbuf b = ofpbuf_const_initializer(fields_data, fields_len); while (b.size) { const uint8_t *pos = b.data; const struct mf_field *field; @@ -1291,15 +1287,12 @@ static void format_nxm_field_name(struct ds *, uint64_t header); char * nx_match_to_string(const uint8_t *p, unsigned int match_len) { - struct ofpbuf b; - struct ds s; - if (!match_len) { return xstrdup(""); } - ofpbuf_use_const(&b, p, match_len); - ds_init(&s); + struct ofpbuf b = ofpbuf_const_initializer(p, match_len); + struct ds s = DS_EMPTY_INITIALIZER; while (b.size) { union mf_value value; union mf_value mask; diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c index 24f18d987..7dc852e6a 100644 --- a/lib/ofp-actions.c +++ b/lib/ofp-actions.c @@ -889,18 +889,16 @@ decode_NXAST_RAW_OUTPUT_REG2(const struct nx_action_output_reg2 *naor, struct ofpbuf *out) { struct ofpact_output_reg *output_reg; - enum ofperr error; - struct ofpbuf b; - output_reg = ofpact_put_OUTPUT_REG(out); output_reg->ofpact.raw = NXAST_RAW_OUTPUT_REG2; output_reg->src.ofs = nxm_decode_ofs(naor->ofs_nbits); output_reg->src.n_bits = nxm_decode_n_bits(naor->ofs_nbits); output_reg->max_len = ntohs(naor->max_len); - ofpbuf_use_const(&b, naor, ntohs(naor->len)); + struct ofpbuf b = ofpbuf_const_initializer(naor, ntohs(naor->len)); ofpbuf_pull(&b, OBJECT_OFFSETOF(naor, pad)); - error = nx_pull_header(&b, &output_reg->src.field, NULL); + + enum ofperr error = nx_pull_header(&b, &output_reg->src.field, NULL); if (error) { return error; } @@ -2011,20 +2009,17 @@ decode_copy_field__(ovs_be16 src_offset, ovs_be16 dst_offset, ovs_be16 n_bits, const void *action, ovs_be16 action_len, size_t oxm_offset, struct ofpbuf *ofpacts) { - struct ofpact_reg_move *move; - enum ofperr error; - struct ofpbuf b; - - move = ofpact_put_REG_MOVE(ofpacts); + struct ofpact_reg_move *move = ofpact_put_REG_MOVE(ofpacts); move->ofpact.raw = ONFACT_RAW13_COPY_FIELD; move->src.ofs = ntohs(src_offset); move->src.n_bits = ntohs(n_bits); move->dst.ofs = ntohs(dst_offset); move->dst.n_bits = ntohs(n_bits); - ofpbuf_use_const(&b, action, ntohs(action_len)); + struct ofpbuf b = ofpbuf_const_initializer(action, ntohs(action_len)); ofpbuf_pull(&b, oxm_offset); - error = nx_pull_header(&b, &move->src.field, NULL); + + enum ofperr error = nx_pull_header(&b, &move->src.field, NULL); if (error) { return error; } @@ -2065,20 +2060,17 @@ decode_NXAST_RAW_REG_MOVE(const struct nx_action_reg_move *narm, enum ofp_version ofp_version OVS_UNUSED, struct ofpbuf *ofpacts) { - struct ofpact_reg_move *move; - enum ofperr error; - struct ofpbuf b; - - move = ofpact_put_REG_MOVE(ofpacts); + struct ofpact_reg_move *move = ofpact_put_REG_MOVE(ofpacts); move->ofpact.raw = NXAST_RAW_REG_MOVE; move->src.ofs = ntohs(narm->src_ofs); move->src.n_bits = ntohs(narm->n_bits); move->dst.ofs = ntohs(narm->dst_ofs); move->dst.n_bits = ntohs(narm->n_bits); - ofpbuf_use_const(&b, narm, ntohs(narm->len)); + struct ofpbuf b = ofpbuf_const_initializer(narm, ntohs(narm->len)); ofpbuf_pull(&b, sizeof *narm); - error = nx_pull_header(&b, &move->src.field, NULL); + + enum ofperr error = nx_pull_header(&b, &move->src.field, NULL); if (error) { return error; } @@ -2242,16 +2234,12 @@ static enum ofperr decode_ofpat_set_field(const struct ofp12_action_set_field *oasf, bool may_mask, struct ofpbuf *ofpacts) { - struct ofpact_set_field *sf; - enum ofperr error; - struct ofpbuf b; - - sf = ofpact_put_SET_FIELD(ofpacts); - - ofpbuf_use_const(&b, oasf, ntohs(oasf->len)); + struct ofpbuf b = ofpbuf_const_initializer(oasf, ntohs(oasf->len)); ofpbuf_pull(&b, OBJECT_OFFSETOF(oasf, pad)); - error = nx_pull_entry(&b, &sf->field, &sf->value, - may_mask ? &sf->mask : NULL); + + struct ofpact_set_field *sf = ofpact_put_SET_FIELD(ofpacts); + enum ofperr error = nx_pull_entry(&b, &sf->field, &sf->value, + may_mask ? &sf->mask : NULL); if (error) { return (error == OFPERR_OFPBMC_BAD_MASK ? OFPERR_OFPBAC_BAD_SET_MASK @@ -2353,16 +2341,13 @@ decode_NXAST_RAW_REG_LOAD2(const struct nx_action_reg_load2 *narl, enum ofp_version ofp_version OVS_UNUSED, struct ofpbuf *out) { - struct ofpact_set_field *sf; - enum ofperr error; - struct ofpbuf b; - - sf = ofpact_put_SET_FIELD(out); + struct ofpact_set_field *sf = ofpact_put_SET_FIELD(out); sf->ofpact.raw = NXAST_RAW_REG_LOAD2; - ofpbuf_use_const(&b, narl, ntohs(narl->len)); + struct ofpbuf b = ofpbuf_const_initializer(narl, ntohs(narl->len)); ofpbuf_pull(&b, OBJECT_OFFSETOF(narl, pad)); - error = nx_pull_entry(&b, &sf->field, &sf->value, &sf->mask); + + enum ofperr error = nx_pull_entry(&b, &sf->field, &sf->value, &sf->mask); if (error) { return error; } @@ -2799,14 +2784,12 @@ static enum ofperr decode_stack_action(const struct nx_action_stack *nasp, struct ofpact_stack *stack_action) { - enum ofperr error; - struct ofpbuf b; - stack_action->subfield.ofs = ntohs(nasp->offset); - ofpbuf_use_const(&b, nasp, sizeof *nasp); + struct ofpbuf b = ofpbuf_const_initializer(nasp, sizeof *nasp); ofpbuf_pull(&b, OBJECT_OFFSETOF(nasp, pad)); - error = nx_pull_header(&b, &stack_action->subfield.field, NULL); + enum ofperr error = nx_pull_header(&b, &stack_action->subfield.field, + NULL); if (error) { return error; } @@ -4791,13 +4774,10 @@ decode_NXAST_RAW_CT(const struct nx_action_conntrack *nac, enum ofp_version ofp_version, struct ofpbuf *out) { const size_t ct_offset = ofpacts_pull(out); - struct ofpact_conntrack *conntrack; - struct ofpbuf openflow; - int error = 0; - - conntrack = ofpact_put_CT(out); + struct ofpact_conntrack *conntrack = ofpact_put_CT(out); conntrack->flags = ntohs(nac->flags); - error = decode_ct_zone(nac, conntrack); + + int error = decode_ct_zone(nac, conntrack); if (error) { goto out; } @@ -4806,7 +4786,8 @@ decode_NXAST_RAW_CT(const struct nx_action_conntrack *nac, ofpbuf_pull(out, sizeof(*conntrack)); - ofpbuf_use_const(&openflow, nac + 1, ntohs(nac->len) - sizeof(*nac)); + struct ofpbuf openflow = ofpbuf_const_initializer( + nac + 1, ntohs(nac->len) - sizeof(*nac)); error = ofpacts_pull_openflow_actions__(&openflow, openflow.size, ofp_version, 1u << OVSINST_OFPIT11_APPLY_ACTIONS, @@ -5584,9 +5565,7 @@ static enum ofperr ofpacts_decode(const void *actions, size_t actions_len, enum ofp_version ofp_version, struct ofpbuf *ofpacts) { - struct ofpbuf openflow; - - ofpbuf_use_const(&openflow, actions, actions_len); + struct ofpbuf openflow = ofpbuf_const_initializer(actions, actions_len); while (openflow.size) { const struct ofp_action_header *action = openflow.data; enum ofp_raw_action_type raw; diff --git a/lib/ofp-errors.c b/lib/ofp-errors.c index 615910520..f9e1fa9b7 100644 --- a/lib/ofp-errors.c +++ b/lib/ofp-errors.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2012, 2013, 2014 Nicira, Inc. + * Copyright (c) 2012, 2013, 2014, 2016 Nicira, Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -284,17 +284,15 @@ ofperr_decode_msg(const struct ofp_header *oh, struct ofpbuf *payload) const struct ofp_error_msg *oem; enum ofpraw raw; uint16_t type, code; - enum ofperr error; uint32_t vendor; - struct ofpbuf b; if (payload) { memset(payload, 0, sizeof *payload); } /* Pull off the error message. */ - ofpbuf_use_const(&b, oh, ntohs(oh->length)); - error = ofpraw_pull(&raw, &b); + struct ofpbuf b = ofpbuf_const_initializer(oh, ntohs(oh->length)); + enum ofperr error = ofpraw_pull(&raw, &b); if (error) { return 0; } diff --git a/lib/ofp-msgs.c b/lib/ofp-msgs.c index bc38f401b..de20655b4 100644 --- a/lib/ofp-msgs.c +++ b/lib/ofp-msgs.c @@ -407,9 +407,7 @@ ofphdrs_len(const struct ofphdrs *hdrs) enum ofperr ofpraw_decode(enum ofpraw *raw, const struct ofp_header *oh) { - struct ofpbuf msg; - - ofpbuf_use_const(&msg, oh, ntohs(oh->length)); + struct ofpbuf msg = ofpbuf_const_initializer(oh, ntohs(oh->length)); return ofpraw_pull(raw, &msg); } diff --git a/lib/ofp-print.c b/lib/ofp-print.c index 5af4bf0b4..3195376fb 100644 --- a/lib/ofp-print.c +++ b/lib/ofp-print.c @@ -455,11 +455,8 @@ static void ofp_print_switch_features(struct ds *string, const struct ofp_header *oh) { struct ofputil_switch_features features; - enum ofperr error; - struct ofpbuf b; - - ofpbuf_use_const(&b, oh, ntohs(oh->length)); - error = ofputil_pull_switch_features(&b, &features); + struct ofpbuf b = ofpbuf_const_initializer(oh, ntohs(oh->length)); + enum ofperr error = ofputil_pull_switch_features(&b, &features); if (error) { ofp_print_error(string, error); return; @@ -1121,11 +1118,9 @@ static void ofp_print_queue_get_config_reply(struct ds *string, const struct ofp_header *oh) { - struct ofpbuf b; + struct ofpbuf b = ofpbuf_const_initializer(oh, ntohs(oh->length)); ofp_port_t port = 0; - ofpbuf_use_const(&b, oh, ntohs(oh->length)); - ds_put_char(string, ' '); for (;;) { struct ofputil_queue_config queue; @@ -1338,10 +1333,9 @@ ofp_print_meter_features_reply(struct ds *s, const struct ofp_header *oh) static void ofp_print_meter_config_reply(struct ds *s, const struct ofp_header *oh) { + struct ofpbuf b = ofpbuf_const_initializer(oh, ntohs(oh->length)); struct ofpbuf bands; - struct ofpbuf b; - ofpbuf_use_const(&b, oh, ntohs(oh->length)); ofpbuf_init(&bands, 64); for (;;) { struct ofputil_meter_config mc; @@ -1363,10 +1357,9 @@ ofp_print_meter_config_reply(struct ds *s, const struct ofp_header *oh) static void ofp_print_meter_stats_reply(struct ds *s, const struct ofp_header *oh) { + struct ofpbuf b = ofpbuf_const_initializer(oh, ntohs(oh->length)); struct ofpbuf bands; - struct ofpbuf b; - ofpbuf_use_const(&b, oh, ntohs(oh->length)); ofpbuf_init(&bands, 64); for (;;) { struct ofputil_meter_stats ms; @@ -1545,10 +1538,9 @@ ofp_print_flow_stats(struct ds *string, struct ofputil_flow_stats *fs) static void ofp_print_flow_stats_reply(struct ds *string, const struct ofp_header *oh) { + struct ofpbuf b = ofpbuf_const_initializer(oh, ntohs(oh->length)); struct ofpbuf ofpacts; - struct ofpbuf b; - ofpbuf_use_const(&b, oh, ntohs(oh->length)); ofpbuf_init(&ofpacts, 64); for (;;) { struct ofputil_flow_stats fs; @@ -1620,14 +1612,12 @@ static void ofp_print_ofpst_port_reply(struct ds *string, const struct ofp_header *oh, int verbosity) { - struct ofpbuf b; - ds_put_format(string, " %"PRIuSIZE" ports\n", ofputil_count_port_stats(oh)); if (verbosity < 1) { return; } - ofpbuf_use_const(&b, oh, ntohs(oh->length)); + struct ofpbuf b = ofpbuf_const_initializer(oh, ntohs(oh->length)); for (;;) { struct ofputil_port_stats ps; int retval; @@ -1673,9 +1663,7 @@ ofp_print_ofpst_port_reply(struct ds *string, const struct ofp_header *oh, static void ofp_print_table_stats_reply(struct ds *string, const struct ofp_header *oh) { - struct ofpbuf b; - - ofpbuf_use_const(&b, oh, ntohs(oh->length)); + struct ofpbuf b = ofpbuf_const_initializer(oh, ntohs(oh->length)); ofpraw_pull_assert(&b); struct ofputil_table_features prev_features; @@ -1735,14 +1723,12 @@ static void ofp_print_ofpst_queue_reply(struct ds *string, const struct ofp_header *oh, int verbosity) { - struct ofpbuf b; - ds_put_format(string, " %"PRIuSIZE" queues\n", ofputil_count_queue_stats(oh)); if (verbosity < 1) { return; } - ofpbuf_use_const(&b, oh, ntohs(oh->length)); + struct ofpbuf b = ofpbuf_const_initializer(oh, ntohs(oh->length)); for (;;) { struct ofputil_queue_stats qs; int retval; @@ -1796,9 +1782,7 @@ static void ofp_print_ofpst_port_desc_reply(struct ds *string, const struct ofp_header *oh) { - struct ofpbuf b; - - ofpbuf_use_const(&b, oh, ntohs(oh->length)); + struct ofpbuf b = ofpbuf_const_initializer(oh, ntohs(oh->length)); ofpraw_pull_assert(&b); ds_put_char(string, '\n'); ofp_print_phy_ports(string, oh->version, &b); @@ -2144,9 +2128,7 @@ static void ofp_print_nxst_flow_monitor_request(struct ds *string, const struct ofp_header *oh) { - struct ofpbuf b; - - ofpbuf_use_const(&b, oh, ntohs(oh->length)); + struct ofpbuf b = ofpbuf_const_initializer(oh, ntohs(oh->length)); for (;;) { struct ofputil_flow_monitor_request request; int retval; @@ -2183,11 +2165,9 @@ ofp_print_nxst_flow_monitor_reply(struct ds *string, const struct ofp_header *oh) { uint64_t ofpacts_stub[1024 / 8]; - struct ofpbuf ofpacts; - struct ofpbuf b; + struct ofpbuf ofpacts = OFPBUF_STUB_INITIALIZER(ofpacts_stub); + struct ofpbuf b = ofpbuf_const_initializer(oh, ntohs(oh->length)); - ofpbuf_use_const(&b, oh, ntohs(oh->length)); - ofpbuf_use_stub(&ofpacts, ofpacts_stub, sizeof ofpacts_stub); for (;;) { char reasonbuf[OFP_FLOW_REMOVED_REASON_BUFSIZE]; struct ofputil_flow_update update; @@ -2389,9 +2369,7 @@ ofp_print_ofpst_group_desc_request(struct ds *string, static void ofp_print_group_desc(struct ds *s, const struct ofp_header *oh) { - struct ofpbuf b; - - ofpbuf_use_const(&b, oh, ntohs(oh->length)); + struct ofpbuf b = ofpbuf_const_initializer(oh, ntohs(oh->length)); for (;;) { struct ofputil_group_desc gd; int retval; @@ -2431,11 +2409,7 @@ ofp_print_ofpst_group_request(struct ds *string, const struct ofp_header *oh) static void ofp_print_group_stats(struct ds *s, const struct ofp_header *oh) { - struct ofpbuf b; - uint32_t bucket_i; - - ofpbuf_use_const(&b, oh, ntohs(oh->length)); - + struct ofpbuf b = ofpbuf_const_initializer(oh, ntohs(oh->length)); for (;;) { struct ofputil_group_stats gs; int retval; @@ -2462,7 +2436,7 @@ ofp_print_group_stats(struct ds *s, const struct ofp_header *oh) ds_put_format(s, "packet_count=%"PRIu64",", gs.packet_count); ds_put_format(s, "byte_count=%"PRIu64"", gs.byte_count); - for (bucket_i = 0; bucket_i < gs.n_buckets; bucket_i++) { + for (uint32_t bucket_i = 0; bucket_i < gs.n_buckets; bucket_i++) { if (gs.bucket_stats[bucket_i].packet_count != UINT64_MAX) { ds_put_format(s, ",bucket%"PRIu32":", bucket_i); ds_put_format(s, "packet_count=%"PRIu64",", gs.bucket_stats[bucket_i].packet_count); @@ -2832,9 +2806,7 @@ ofp_print_table_features(struct ds *s, static void ofp_print_table_features_reply(struct ds *s, const struct ofp_header *oh) { - struct ofpbuf b; - - ofpbuf_use_const(&b, oh, ntohs(oh->length)); + struct ofpbuf b = ofpbuf_const_initializer(oh, ntohs(oh->length)); struct ofputil_table_features prev; for (int i = 0; ; i++) { @@ -2858,10 +2830,7 @@ ofp_print_table_features_reply(struct ds *s, const struct ofp_header *oh) static void ofp_print_table_desc_reply(struct ds *s, const struct ofp_header *oh) { - struct ofpbuf b; - - ofpbuf_use_const(&b, oh, ntohs(oh->length)); - + struct ofpbuf b = ofpbuf_const_initializer(oh, ntohs(oh->length)); for (;;) { struct ofputil_table_desc td; int retval; diff --git a/lib/ofp-util.c b/lib/ofp-util.c index aa4d2f3ca..23f7eda28 100644 --- a/lib/ofp-util.c +++ b/lib/ofp-util.c @@ -1244,13 +1244,12 @@ version_bitmap_from_version(uint8_t ofp_version) bool ofputil_decode_hello(const struct ofp_header *oh, uint32_t *allowed_versions) { - struct ofpbuf msg; - bool ok = true; - - ofpbuf_use_const(&msg, oh, ntohs(oh->length)); + struct ofpbuf msg = ofpbuf_const_initializer(oh, ntohs(oh->length)); ofpbuf_pull(&msg, sizeof *oh); *allowed_versions = version_bitmap_from_version(oh->version); + + bool ok = true; while (msg.size) { const struct ofp_hello_elem_header *oheh; unsigned int len; @@ -1545,14 +1544,12 @@ ofputil_decode_flow_mod(struct ofputil_flow_mod *fm, { ovs_be16 raw_flags; enum ofperr error; - struct ofpbuf b; - enum ofpraw raw; /* Ignored for non-delete actions */ fm->delete_reason = OFPRR_DELETE; - ofpbuf_use_const(&b, oh, ntohs(oh->length)); - raw = ofpraw_pull_assert(&b); + struct ofpbuf b = ofpbuf_const_initializer(oh, ntohs(oh->length)); + enum ofpraw raw = ofpraw_pull_assert(&b); if (raw == OFPRAW_OFPT11_FLOW_MOD) { /* Standard OpenFlow 1.1+ flow_mod. */ const struct ofp11_flow_mod *ofm; @@ -1773,12 +1770,9 @@ ofputil_decode_meter_mod(const struct ofp_header *oh, struct ofputil_meter_mod *mm, struct ofpbuf *bands) { - const struct ofp13_meter_mod *omm; - struct ofpbuf b; - - ofpbuf_use_const(&b, oh, ntohs(oh->length)); + struct ofpbuf b = ofpbuf_const_initializer(oh, ntohs(oh->length)); ofpraw_pull_assert(&b); - omm = ofpbuf_pull(&b, sizeof *omm); + const struct ofp13_meter_mod *omm = ofpbuf_pull(&b, sizeof *omm); /* Translate the message. */ mm->command = ntohs(omm->command); @@ -2342,11 +2336,8 @@ ofputil_decode_queue_get_config_request(const struct ofp_header *oh, const struct ofp10_queue_get_config_request *qgcr10; const struct ofp11_queue_get_config_request *qgcr11; const struct ofp14_queue_desc_request *qdr14; - enum ofpraw raw; - struct ofpbuf b; - - ofpbuf_use_const(&b, oh, ntohs(oh->length)); - raw = ofpraw_pull_assert(&b); + struct ofpbuf b = ofpbuf_const_initializer(oh, ntohs(oh->length)); + enum ofpraw raw = ofpraw_pull_assert(&b); switch ((int) raw) { case OFPRAW_OFPT10_QUEUE_GET_CONFIG_REQUEST: @@ -2614,8 +2605,8 @@ ofputil_pull_queue_get_config_reply14(struct ofpbuf *msg, } len -= sizeof *oqd14; - struct ofpbuf properties; - ofpbuf_use_const(&properties, ofpbuf_pull(msg, len), len); + struct ofpbuf properties = ofpbuf_const_initializer(ofpbuf_pull(msg, len), + len); while (properties.size > 0) { struct ofpbuf payload; uint64_t type; @@ -2695,11 +2686,8 @@ enum ofperr ofputil_decode_flow_stats_request(struct ofputil_flow_stats_request *fsr, const struct ofp_header *oh) { - enum ofpraw raw; - struct ofpbuf b; - - ofpbuf_use_const(&b, oh, ntohs(oh->length)); - raw = ofpraw_pull_assert(&b); + struct ofpbuf b = ofpbuf_const_initializer(oh, ntohs(oh->length)); + enum ofpraw raw = ofpraw_pull_assert(&b); switch ((int) raw) { case OFPRAW_OFPST10_FLOW_REQUEST: return ofputil_decode_ofpst10_flow_request(fsr, b.data, false); @@ -3127,13 +3115,10 @@ enum ofperr ofputil_decode_aggregate_stats_reply(struct ofputil_aggregate_stats *stats, const struct ofp_header *reply) { - struct ofp_aggregate_stats_reply *asr; - struct ofpbuf msg; - - ofpbuf_use_const(&msg, reply, ntohs(reply->length)); + struct ofpbuf msg = ofpbuf_const_initializer(reply, ntohs(reply->length)); ofpraw_pull_assert(&msg); - asr = msg.msg; + struct ofp_aggregate_stats_reply *asr = msg.msg; stats->packet_count = ntohll(get_32aligned_be64(&asr->packet_count)); stats->byte_count = ntohll(get_32aligned_be64(&asr->byte_count)); stats->flow_count = ntohl(asr->flow_count); @@ -3148,11 +3133,8 @@ enum ofperr ofputil_decode_flow_removed(struct ofputil_flow_removed *fr, const struct ofp_header *oh) { - enum ofpraw raw; - struct ofpbuf b; - - ofpbuf_use_const(&b, oh, ntohs(oh->length)); - raw = ofpraw_pull_assert(&b); + struct ofpbuf b = ofpbuf_const_initializer(oh, ntohs(oh->length)); + enum ofpraw raw = ofpraw_pull_assert(&b); if (raw == OFPRAW_OFPT11_FLOW_REMOVED) { const struct ofp12_flow_removed *ofr; enum ofperr error; @@ -3325,14 +3307,11 @@ ofputil_decode_packet_in(const struct ofp_header *oh, struct ofputil_packet_in *pin, size_t *total_len, uint32_t *buffer_id) { - enum ofpraw raw; - struct ofpbuf b; - memset(pin, 0, sizeof *pin); pin->cookie = OVS_BE64_MAX; - ofpbuf_use_const(&b, oh, ntohs(oh->length)); - raw = ofpraw_pull_assert(&b); + struct ofpbuf b = ofpbuf_const_initializer(oh, ntohs(oh->length)); + enum ofpraw raw = ofpraw_pull_assert(&b); if (raw == OFPRAW_OFPT13_PACKET_IN || raw == OFPRAW_OFPT12_PACKET_IN) { const struct ofp12_packet_in *opi = ofpbuf_pull(&b, sizeof *opi); const ovs_be64 *cookie = (raw == OFPRAW_OFPT13_PACKET_IN @@ -3671,11 +3650,8 @@ ofputil_decode_packet_out(struct ofputil_packet_out *po, const struct ofp_header *oh, struct ofpbuf *ofpacts) { - enum ofpraw raw; - struct ofpbuf b; - - ofpbuf_use_const(&b, oh, ntohs(oh->length)); - raw = ofpraw_pull_assert(&b); + struct ofpbuf b = ofpbuf_const_initializer(oh, ntohs(oh->length)); + enum ofpraw raw = ofpraw_pull_assert(&b); ofpbuf_clear(ofpacts); if (raw == OFPRAW_OFPT11_PACKET_OUT) { @@ -3864,24 +3840,18 @@ parse_ofp14_port_ethernet_property(const struct ofpbuf *payload, static enum ofperr ofputil_pull_ofp14_port(struct ofputil_phy_port *pp, struct ofpbuf *msg) { - struct ofpbuf properties; - struct ofp14_port *op; - enum ofperr error; - size_t len; - - op = ofpbuf_try_pull(msg, sizeof *op); + struct ofp14_port *op = ofpbuf_try_pull(msg, sizeof *op); if (!op) { return OFPERR_OFPBRC_BAD_LEN; } - len = ntohs(op->length); + size_t len = ntohs(op->length); if (len < sizeof *op || len - sizeof *op > msg->size) { return OFPERR_OFPBRC_BAD_LEN; } len -= sizeof *op; - ofpbuf_use_const(&properties, ofpbuf_pull(msg, len), len); - error = ofputil_port_from_ofp11(op->port_no, &pp->port_no); + enum ofperr error = ofputil_port_from_ofp11(op->port_no, &pp->port_no); if (error) { return error; } @@ -3891,6 +3861,8 @@ ofputil_pull_ofp14_port(struct ofputil_phy_port *pp, struct ofpbuf *msg) pp->config = ntohl(op->config) & OFPPC11_ALL; pp->state = ntohl(op->state) & OFPPS11_ALL; + struct ofpbuf properties = ofpbuf_const_initializer(ofpbuf_pull(msg, len), + len); while (properties.size > 0) { struct ofpbuf payload; enum ofperr error; @@ -4019,11 +3991,9 @@ enum ofperr ofputil_decode_port_desc_stats_request(const struct ofp_header *request, ofp_port_t *port) { - struct ofpbuf b; - enum ofpraw raw; - - ofpbuf_use_const(&b, request, ntohs(request->length)); - raw = ofpraw_pull_assert(&b); + struct ofpbuf b = ofpbuf_const_initializer(request, + ntohs(request->length)); + enum ofpraw raw = ofpraw_pull_assert(&b); if (raw == OFPRAW_OFPST10_PORT_DESC_REQUEST) { *port = OFPP_ANY; return 0; @@ -4087,13 +4057,10 @@ static bool ofputil_decode_switch_config(const struct ofp_header *oh, struct ofputil_switch_config *config) { - const struct ofp_switch_config *osc; - struct ofpbuf b; - - ofpbuf_use_const(&b, oh, ntohs(oh->length)); + struct ofpbuf b = ofpbuf_const_initializer(oh, ntohs(oh->length)); ofpraw_pull_assert(&b); - osc = ofpbuf_pull(&b, sizeof *osc); + const struct ofp_switch_config *osc = ofpbuf_pull(&b, sizeof *osc); config->frag = ntohs(osc->flags) & OFPC_FRAG_MASK; config->miss_send_len = ntohs(osc->miss_send_len); @@ -4362,14 +4329,10 @@ enum ofperr ofputil_decode_port_status(const struct ofp_header *oh, struct ofputil_port_status *ps) { - const struct ofp_port_status *ops; - struct ofpbuf b; - int retval; - - ofpbuf_use_const(&b, oh, ntohs(oh->length)); + struct ofpbuf b = ofpbuf_const_initializer(oh, ntohs(oh->length)); ofpraw_pull_assert(&b); - ops = ofpbuf_pull(&b, sizeof *ops); + const struct ofp_port_status *ops = ofpbuf_pull(&b, sizeof *ops); if (ops->reason != OFPPR_ADD && ops->reason != OFPPR_DELETE && ops->reason != OFPPR_MODIFY) { @@ -4377,7 +4340,7 @@ ofputil_decode_port_status(const struct ofp_header *oh, } ps->reason = ops->reason; - retval = ofputil_pull_phy_port(oh->version, &b, &ps->desc); + int retval = ofputil_pull_phy_port(oh->version, &b, &ps->desc); ovs_assert(retval != EOF); return retval; } @@ -4445,12 +4408,8 @@ enum ofperr ofputil_decode_port_mod(const struct ofp_header *oh, struct ofputil_port_mod *pm, bool loose) { - enum ofpraw raw; - struct ofpbuf b; - - ofpbuf_use_const(&b, oh, ntohs(oh->length)); - raw = ofpraw_pull_assert(&b); - + struct ofpbuf b = ofpbuf_const_initializer(oh, ntohs(oh->length)); + enum ofpraw raw = ofpraw_pull_assert(&b); if (raw == OFPRAW_OFPT10_PORT_MOD) { const struct ofp10_port_mod *opm = b.data; @@ -4721,33 +4680,26 @@ int ofputil_decode_table_features(struct ofpbuf *msg, struct ofputil_table_features *tf, bool loose) { - const struct ofp_header *oh; - struct ofp13_table_features *otf; - struct ofpbuf properties; - unsigned int len; - memset(tf, 0, sizeof *tf); if (!msg->header) { ofpraw_pull_assert(msg); } - oh = msg->header; if (!msg->size) { return EOF; } + const struct ofp_header *oh = msg->header; + struct ofp13_table_features *otf = msg->data; if (msg->size < sizeof *otf) { return OFPERR_OFPBPC_BAD_LEN; } - otf = msg->data; - len = ntohs(otf->length); + unsigned int len = ntohs(otf->length); if (len < sizeof *otf || len % 8 || len > msg->size) { return OFPERR_OFPBPC_BAD_LEN; } - ofpbuf_use_const(&properties, ofpbuf_pull(msg, len), len); - ofpbuf_pull(&properties, sizeof *otf); tf->table_id = otf->table_id; if (tf->table_id == OFPTT_ALL) { @@ -4768,6 +4720,9 @@ ofputil_decode_table_features(struct ofpbuf *msg, } tf->max_entries = ntohl(otf->max_entries); + struct ofpbuf properties = ofpbuf_const_initializer(ofpbuf_pull(msg, len), + len); + ofpbuf_pull(&properties, sizeof *otf); while (properties.size > 0) { struct ofpbuf payload; enum ofperr error; @@ -5012,10 +4967,6 @@ ofputil_decode_table_desc(struct ofpbuf *msg, struct ofputil_table_desc *td, enum ofp_version version) { - struct ofp14_table_desc *otd; - struct ofpbuf properties; - size_t length; - memset(td, 0, sizeof *td); if (!msg->header) { @@ -5026,7 +4977,7 @@ ofputil_decode_table_desc(struct ofpbuf *msg, return EOF; } - otd = ofpbuf_try_pull(msg, sizeof *otd); + struct ofp14_table_desc *otd = ofpbuf_try_pull(msg, sizeof *otd); if (!otd) { VLOG_WARN_RL(&bad_ofmsg_rl, "OFP14_TABLE_DESC reply has %"PRIu32" " "leftover bytes at end", msg->size); @@ -5034,19 +4985,20 @@ ofputil_decode_table_desc(struct ofpbuf *msg, } td->table_id = otd->table_id; - length = ntohs(otd->length); + size_t length = ntohs(otd->length); if (length < sizeof *otd || length - sizeof *otd > msg->size) { VLOG_WARN_RL(&bad_ofmsg_rl, "OFP14_TABLE_DESC reply claims invalid " "length %"PRIuSIZE, length); return OFPERR_OFPBRC_BAD_LEN; } length -= sizeof *otd; - ofpbuf_use_const(&properties, ofpbuf_pull(msg, length), length); td->eviction = ofputil_decode_table_eviction(otd->config, version); td->vacancy = ofputil_decode_table_vacancy(otd->config, version); td->eviction_flags = UINT32_MAX; + struct ofpbuf properties = ofpbuf_const_initializer( + ofpbuf_pull(msg, length), length); while (properties.size > 0) { struct ofpbuf payload; enum ofperr error; @@ -5290,17 +5242,14 @@ enum ofperr ofputil_decode_table_mod(const struct ofp_header *oh, struct ofputil_table_mod *pm) { - enum ofpraw raw; - struct ofpbuf b; - memset(pm, 0, sizeof *pm); pm->miss = OFPUTIL_TABLE_MISS_DEFAULT; pm->eviction = OFPUTIL_TABLE_EVICTION_DEFAULT; pm->eviction_flags = UINT32_MAX; pm->vacancy = OFPUTIL_TABLE_VACANCY_DEFAULT; - ofpbuf_use_const(&b, oh, ntohs(oh->length)); - raw = ofpraw_pull_assert(&b); + struct ofpbuf b = ofpbuf_const_initializer(oh, ntohs(oh->length)); + enum ofpraw raw = ofpraw_pull_assert(&b); if (raw == OFPRAW_OFPT11_TABLE_MOD) { const struct ofp11_table_mod *otm = b.data; @@ -5414,12 +5363,8 @@ enum ofperr ofputil_decode_role_message(const struct ofp_header *oh, struct ofputil_role_request *rr) { - struct ofpbuf b; - enum ofpraw raw; - - ofpbuf_use_const(&b, oh, ntohs(oh->length)); - raw = ofpraw_pull_assert(&b); - + struct ofpbuf b = ofpbuf_const_initializer(oh, ntohs(oh->length)); + enum ofpraw raw = ofpraw_pull_assert(&b); if (raw == OFPRAW_OFPT12_ROLE_REQUEST || raw == OFPRAW_OFPT12_ROLE_REPLY) { const struct ofp12_role_request *orr = b.msg; @@ -5533,15 +5478,11 @@ enum ofperr ofputil_decode_role_status(const struct ofp_header *oh, struct ofputil_role_status *rs) { - struct ofpbuf b; - enum ofpraw raw; - const struct ofp14_role_status *r; - - ofpbuf_use_const(&b, oh, ntohs(oh->length)); - raw = ofpraw_pull_assert(&b); + struct ofpbuf b = ofpbuf_const_initializer(oh, ntohs(oh->length)); + enum ofpraw raw = ofpraw_pull_assert(&b); ovs_assert(raw == OFPRAW_OFPT14_ROLE_STATUS); - r = b.msg; + const struct ofp14_role_status *r = b.msg; if (r->role != htonl(OFPCR12_ROLE_NOCHANGE) && r->role != htonl(OFPCR12_ROLE_EQUAL) && r->role != htonl(OFPCR12_ROLE_MASTER) && @@ -5600,10 +5541,7 @@ enum ofperr ofputil_decode_requestforward(const struct ofp_header *outer, struct ofputil_requestforward *rf) { - struct ofpbuf b; - enum ofperr error; - - ofpbuf_use_const(&b, outer, ntohs(outer->length)); + struct ofpbuf b = ofpbuf_const_initializer(outer, ntohs(outer->length)); /* Skip past outer message. */ enum ofpraw outer_raw = ofpraw_pull_assert(&b); @@ -5624,7 +5562,7 @@ ofputil_decode_requestforward(const struct ofp_header *outer, /* Parse inner message. */ enum ofptype type; - error = ofptype_decode(&type, inner); + enum ofperr error = ofptype_decode(&type, inner); if (error) { return error; } @@ -6434,13 +6372,11 @@ make_echo_request(enum ofp_version ofp_version) struct ofpbuf * make_echo_reply(const struct ofp_header *rq) { - struct ofpbuf rq_buf; - struct ofpbuf *reply; - - ofpbuf_use_const(&rq_buf, rq, ntohs(rq->length)); + struct ofpbuf rq_buf = ofpbuf_const_initializer(rq, ntohs(rq->length)); ofpraw_pull_assert(&rq_buf); - reply = ofpraw_alloc_reply(OFPRAW_OFPT_ECHO_REPLY, rq, rq_buf.size); + struct ofpbuf *reply = ofpraw_alloc_reply(OFPRAW_OFPT_ECHO_REPLY, + rq, rq_buf.size); ofpbuf_put(reply, rq_buf.data, rq_buf.size); return reply; } @@ -7189,24 +7125,18 @@ static enum ofperr ofputil_pull_ofp14_port_stats(struct ofputil_port_stats *ops, struct ofpbuf *msg) { - const struct ofp14_port_stats *ps14; - struct ofpbuf properties; - enum ofperr error; - size_t len; - - ps14 = ofpbuf_try_pull(msg, sizeof *ps14); + const struct ofp14_port_stats *ps14 = ofpbuf_try_pull(msg, sizeof *ps14); if (!ps14) { return OFPERR_OFPBRC_BAD_LEN; } - len = ntohs(ps14->length); + size_t len = ntohs(ps14->length); if (len < sizeof *ps14 || len - sizeof *ps14 > msg->size) { return OFPERR_OFPBRC_BAD_LEN; } len -= sizeof *ps14; - ofpbuf_use_const(&properties, ofpbuf_pull(msg, len), len); - error = ofputil_port_from_ofp11(ps14->port_no, &ops->port_no); + enum ofperr error = ofputil_port_from_ofp11(ps14->port_no, &ops->port_no); if (error) { return error; } @@ -7226,6 +7156,8 @@ ofputil_pull_ofp14_port_stats(struct ofputil_port_stats *ops, ops->stats.rx_crc_errors = UINT64_MAX; ops->stats.collisions = UINT64_MAX; + struct ofpbuf properties = ofpbuf_const_initializer(ofpbuf_pull(msg, len), + len); while (properties.size > 0) { struct ofpbuf payload; enum ofperr error; @@ -7259,16 +7191,15 @@ ofputil_pull_ofp14_port_stats(struct ofputil_port_stats *ops, size_t ofputil_count_port_stats(const struct ofp_header *oh) { - struct ofputil_port_stats ps; - struct ofpbuf b; - size_t n = 0; - - ofpbuf_use_const(&b, oh, ntohs(oh->length)); + struct ofpbuf b = ofpbuf_const_initializer(oh, ntohs(oh->length)); ofpraw_pull_assert(&b); - while (!ofputil_decode_port_stats(&ps, &b)) { - n++; + + for (size_t n = 0; ; n++) { + struct ofputil_port_stats ps; + if (ofputil_decode_port_stats(&ps, &b)) { + return n; + } } - return n; } /* Converts an OFPST_PORT_STATS reply in 'msg' into an abstract @@ -7518,11 +7449,8 @@ ofputil_uninit_group_desc(struct ofputil_group_desc *gd) uint32_t ofputil_decode_group_desc_request(const struct ofp_header *oh) { - struct ofpbuf request; - enum ofpraw raw; - - ofpbuf_use_const(&request, oh, ntohs(oh->length)); - raw = ofpraw_pull_assert(&request); + struct ofpbuf request = ofpbuf_const_initializer(oh, ntohs(oh->length)); + enum ofpraw raw = ofpraw_pull_assert(&request); if (raw == OFPRAW_OFPST11_GROUP_DESC_REQUEST) { return OFPG_ALL; } else if (raw == OFPRAW_OFPST15_GROUP_DESC_REQUEST) { @@ -8052,7 +7980,6 @@ ofputil_pull_ofp15_buckets(struct ofpbuf *msg, size_t buckets_length, struct ofputil_bucket *bucket = NULL; struct ofpbuf ofpacts; enum ofperr err = OFPERR_OFPGMFC_BAD_BUCKET; - struct ofpbuf properties; size_t ob_len, actions_len, properties_len; ovs_be32 watch_port = ofputil_port_to_ofp11(OFPP_ANY); ovs_be32 watch_group = htonl(OFPG_ANY); @@ -8095,9 +8022,8 @@ ofputil_pull_ofp15_buckets(struct ofpbuf *msg, size_t buckets_length, } properties_len = ob_len - sizeof *ob - actions_len; - ofpbuf_use_const(&properties, ofpbuf_pull(msg, properties_len), - properties_len); - + struct ofpbuf properties = ofpbuf_const_initializer( + ofpbuf_pull(msg, properties_len), properties_len); while (properties.size > 0) { struct ofpbuf payload; uint64_t type; @@ -8269,11 +8195,8 @@ parse_ofp15_group_properties(struct ofpbuf *msg, struct ofputil_group_props *gp, size_t properties_len) { - struct ofpbuf properties; - - ofpbuf_use_const(&properties, ofpbuf_pull(msg, properties_len), - properties_len); - + struct ofpbuf properties = ofpbuf_const_initializer( + ofpbuf_pull(msg, properties_len), properties_len); while (properties.size > 0) { struct ofpbuf payload; enum ofperr error; @@ -8692,16 +8615,13 @@ enum ofperr ofputil_decode_group_mod(const struct ofp_header *oh, struct ofputil_group_mod *gm) { - enum ofp_version ofp_version = oh->version; - struct ofpbuf msg; - struct ofputil_bucket *bucket; - enum ofperr err; + ofputil_init_group_properties(&gm->props); - ofpbuf_use_const(&msg, oh, ntohs(oh->length)); + enum ofp_version ofp_version = oh->version; + struct ofpbuf msg = ofpbuf_const_initializer(oh, ntohs(oh->length)); ofpraw_pull_assert(&msg); - ofputil_init_group_properties(&gm->props); - + enum ofperr err; switch (ofp_version) { case OFP11_VERSION: @@ -8719,7 +8639,6 @@ ofputil_decode_group_mod(const struct ofp_header *oh, default: OVS_NOT_REACHED(); } - if (err) { return err; } @@ -8753,6 +8672,7 @@ ofputil_decode_group_mod(const struct ofp_header *oh, return OFPERR_OFPGMFC_BAD_COMMAND; } + struct ofputil_bucket *bucket; LIST_FOR_EACH (bucket, list_node, &gm->buckets) { if (bucket->weight && gm->type != OFPGT11_SELECT) { return OFPERR_OFPGMFC_INVALID_GROUP; @@ -8859,16 +8779,15 @@ ofputil_encode_queue_stats_request(enum ofp_version ofp_version, size_t ofputil_count_queue_stats(const struct ofp_header *oh) { - struct ofputil_queue_stats qs; - struct ofpbuf b; - size_t n = 0; - - ofpbuf_use_const(&b, oh, ntohs(oh->length)); + struct ofpbuf b = ofpbuf_const_initializer(oh, ntohs(oh->length)); ofpraw_pull_assert(&b); - while (!ofputil_decode_queue_stats(&qs, &b)) { - n++; + + for (size_t n = 0; ; n++) { + struct ofputil_queue_stats qs; + if (ofputil_decode_queue_stats(&qs, &b)) { + return n; + } } - return n; } static enum ofperr @@ -9089,15 +9008,11 @@ enum ofperr ofputil_decode_bundle_ctrl(const struct ofp_header *oh, struct ofputil_bundle_ctrl_msg *msg) { - struct ofpbuf b; - enum ofpraw raw; - const struct ofp14_bundle_ctrl_msg *m; - - ofpbuf_use_const(&b, oh, ntohs(oh->length)); - raw = ofpraw_pull_assert(&b); + struct ofpbuf b = ofpbuf_const_initializer(oh, ntohs(oh->length)); + enum ofpraw raw = ofpraw_pull_assert(&b); ovs_assert(raw == OFPRAW_OFPT14_BUNDLE_CONTROL); - m = b.msg; + const struct ofp14_bundle_ctrl_msg *m = b.msg; msg->bundle_id = ntohl(m->bundle_id); msg->type = ntohs(m->type); msg->flags = ntohs(m->flags); @@ -9247,20 +9162,13 @@ ofputil_is_bundlable(enum ofptype type) enum ofperr ofputil_decode_bundle_add(const struct ofp_header *oh, struct ofputil_bundle_add_msg *msg, - enum ofptype *type_ptr) + enum ofptype *typep) { - const struct ofp14_bundle_ctrl_msg *m; - struct ofpbuf b; - enum ofpraw raw; - size_t inner_len; - enum ofperr error; - enum ofptype type; - - ofpbuf_use_const(&b, oh, ntohs(oh->length)); - raw = ofpraw_pull_assert(&b); + struct ofpbuf b = ofpbuf_const_initializer(oh, ntohs(oh->length)); + enum ofpraw raw = ofpraw_pull_assert(&b); ovs_assert(raw == OFPRAW_OFPT14_BUNDLE_ADD_MESSAGE); - m = ofpbuf_pull(&b, sizeof *m); + const struct ofp14_bundle_ctrl_msg *m = ofpbuf_pull(&b, sizeof *m); msg->bundle_id = ntohl(m->bundle_id); msg->flags = ntohs(m->flags); @@ -9268,7 +9176,7 @@ ofputil_decode_bundle_add(const struct ofp_header *oh, if (msg->msg->version != oh->version) { return OFPERR_NXBFC_BAD_VERSION; } - inner_len = ntohs(msg->msg->length); + size_t inner_len = ntohs(msg->msg->length); if (inner_len < sizeof(struct ofp_header) || inner_len > b.size) { return OFPERR_OFPBFC_MSG_BAD_LEN; } @@ -9277,21 +9185,22 @@ ofputil_decode_bundle_add(const struct ofp_header *oh, } /* Reject unbundlable messages. */ - if (!type_ptr) { - type_ptr = &type; - } - error = ofptype_decode(type_ptr, msg->msg); + enum ofptype type; + enum ofperr error = ofptype_decode(&type, msg->msg); if (error) { VLOG_WARN_RL(&bad_ofmsg_rl, "OFPT14_BUNDLE_ADD_MESSAGE contained " "message is unparsable (%s)", ofperr_get_name(error)); return OFPERR_OFPBFC_MSG_UNSUP; /* 'error' would be confusing. */ } - if (!ofputil_is_bundlable(*type_ptr)) { + if (!ofputil_is_bundlable(type)) { VLOG_WARN_RL(&bad_ofmsg_rl, "%s message not allowed inside " - "OFPT14_BUNDLE_ADD_MESSAGE", ofptype_get_name(*type_ptr)); + "OFPT14_BUNDLE_ADD_MESSAGE", ofptype_get_name(type)); return OFPERR_OFPBFC_MSG_UNSUP; } + if (typep) { + *typep = type; + } return 0; } @@ -9389,13 +9298,10 @@ enum ofperr ofputil_decode_tlv_table_mod(const struct ofp_header *oh, struct ofputil_tlv_table_mod *ttm) { - struct ofpbuf msg; - struct nx_tlv_table_mod *nx_ttm; - - ofpbuf_use_const(&msg, oh, ntohs(oh->length)); + struct ofpbuf msg = ofpbuf_const_initializer(oh, ntohs(oh->length)); ofpraw_pull_assert(&msg); - nx_ttm = ofpbuf_pull(&msg, sizeof *nx_ttm); + struct nx_tlv_table_mod *nx_ttm = ofpbuf_pull(&msg, sizeof *nx_ttm); ttm->command = ntohs(nx_ttm->command); if (ttm->command > NXTTMC_CLEAR) { VLOG_WARN_RL(&bad_ofmsg_rl, @@ -9435,13 +9341,10 @@ enum ofperr ofputil_decode_tlv_table_reply(const struct ofp_header *oh, struct ofputil_tlv_table_reply *ttr) { - struct ofpbuf msg; - struct nx_tlv_table_reply *nx_ttr; - - ofpbuf_use_const(&msg, oh, ntohs(oh->length)); + struct ofpbuf msg = ofpbuf_const_initializer(oh, ntohs(oh->length)); ofpraw_pull_assert(&msg); - nx_ttr = ofpbuf_pull(&msg, sizeof *nx_ttr); + struct nx_tlv_table_reply *nx_ttr = ofpbuf_pull(&msg, sizeof *nx_ttr); ttr->max_option_space = ntohl(nx_ttr->max_option_space); ttr->max_fields = ntohs(nx_ttr->max_fields); @@ -9645,11 +9548,8 @@ ofputil_decode_set_async_config(const struct ofp_header *oh, bool loose, const struct ofputil_async_cfg *basis, struct ofputil_async_cfg *ac) { - enum ofpraw raw; - struct ofpbuf b; - - ofpbuf_use_const(&b, oh, ntohs(oh->length)); - raw = ofpraw_pull_assert(&b); + struct ofpbuf b = ofpbuf_const_initializer(oh, ntohs(oh->length)); + enum ofpraw raw = ofpraw_pull_assert(&b); if (raw == OFPRAW_OFPT13_SET_ASYNC || raw == OFPRAW_NXT_SET_ASYNC_CONFIG || diff --git a/lib/ofpbuf.h b/lib/ofpbuf.h index dd72cde02..489a5432f 100644 --- a/lib/ofpbuf.h +++ b/lib/ofpbuf.h @@ -64,8 +64,9 @@ struct ofpbuf { enum ofpbuf_source source; /* Source of memory allocated as 'base'. */ }; -/* An initializer for a struct ofpbuf that will be initially empty and - * uses the space in STUB (which should be an array) as a stub. +/* An initializer for a struct ofpbuf that will be initially empty and uses the + * space in STUB (which should be an array) as a stub. This is the initializer + * form of ofpbuf_use_stub(). * * Usage example: * @@ -83,6 +84,27 @@ struct ofpbuf { .source = OFPBUF_STUB, \ } +/* An initializer for a struct ofpbuf whose data starts at DATA and continues + * for SIZE bytes. This is appropriate for an ofpbuf that will be used to + * inspect existing data, without moving it around or reallocating it, and + * generally without modifying it at all. This is the initializer form of + * ofpbuf_use_const(). + */ +static inline struct ofpbuf +ofpbuf_const_initializer(const void *data, size_t size) +{ + return (struct ofpbuf) { + .base = CONST_CAST(void *, data), + .data = CONST_CAST(void *, data), + .size = size, + .allocated = size, + .header = NULL, + .msg = NULL, + .list_node = OVS_LIST_POISON, + .source = OFPBUF_STACK, + }; +} + void ofpbuf_use_ds(struct ofpbuf *, const struct ds *); void ofpbuf_use_stack(struct ofpbuf *, void *, size_t); void ofpbuf_use_stub(struct ofpbuf *, void *, size_t); diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c index 37f287946..e7b37a987 100644 --- a/ofproto/ofproto-dpif-xlate.c +++ b/ofproto/ofproto-dpif-xlate.c @@ -3335,19 +3335,17 @@ static void xlate_group_bucket(struct xlate_ctx *ctx, struct ofputil_bucket *bucket) { uint64_t action_list_stub[1024 / 8]; - struct ofpbuf action_list, action_set; + struct ofpbuf action_list = OFPBUF_STUB_INITIALIZER(action_list_stub); + struct ofpbuf action_set = ofpbuf_const_initializer(bucket->ofpacts, + bucket->ofpacts_len); struct flow old_flow = ctx->xin->flow; bool old_was_mpls = ctx->was_mpls; - ofpbuf_use_const(&action_set, bucket->ofpacts, bucket->ofpacts_len); - ofpbuf_use_stub(&action_list, action_list_stub, sizeof action_list_stub); - ofpacts_execute_action_set(&action_list, &action_set); ctx->recurse++; do_xlate_actions(action_list.data, action_list.size, ctx); ctx->recurse--; - ofpbuf_uninit(&action_set); ofpbuf_uninit(&action_list); /* Check if need to recirculate. */ diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c index e1efedbbe..c0647b0b6 100644 --- a/ofproto/ofproto.c +++ b/ofproto/ofproto.c @@ -3549,21 +3549,19 @@ handle_table_features_request(struct ofconn *ofconn, const struct ofp_header *request) { struct ofproto *ofproto = ofconn_get_ofproto(ofconn); - struct ofputil_table_features *features; - struct ovs_list replies; - struct ofpbuf msg; - size_t i; - - ofpbuf_use_const(&msg, request, ntohs(request->length)); + struct ofpbuf msg = ofpbuf_const_initializer(request, + ntohs(request->length)); ofpraw_pull_assert(&msg); if (msg.size || ofpmp_more(request)) { return OFPERR_OFPTFFC_EPERM; } + struct ofputil_table_features *features; query_tables(ofproto, &features, NULL); + struct ovs_list replies; ofpmp_init(&replies, request); - for (i = 0; i < ofproto->n_tables; i++) { + for (size_t i = 0; i < ofproto->n_tables; i++) { if (!(ofproto->tables[i].flags & OFTABLE_HIDDEN)) { ofputil_append_table_features_reply(&features[i], &replies); } @@ -5602,17 +5600,14 @@ handle_flow_monitor_request(struct ofconn *ofconn, const struct ofp_header *oh) OVS_EXCLUDED(ofproto_mutex) { struct ofproto *ofproto = ofconn_get_ofproto(ofconn); - struct ofmonitor **monitors; - size_t n_monitors, allocated_monitors; - struct rule_collection rules; - struct ovs_list replies; - enum ofperr error; - struct ofpbuf b; - size_t i; - ofpbuf_use_const(&b, oh, ntohs(oh->length)); - monitors = NULL; - n_monitors = allocated_monitors = 0; + struct ofpbuf b = ofpbuf_const_initializer(oh, ntohs(oh->length)); + + struct ofmonitor **monitors = NULL; + size_t allocated_monitors = 0; + size_t n_monitors = 0; + + enum ofperr error; ovs_mutex_lock(&ofproto_mutex); for (;;) { @@ -5646,11 +5641,13 @@ handle_flow_monitor_request(struct ofconn *ofconn, const struct ofp_header *oh) monitors[n_monitors++] = m; } + struct rule_collection rules; rule_collection_init(&rules); - for (i = 0; i < n_monitors; i++) { + for (size_t i = 0; i < n_monitors; i++) { ofproto_collect_ofmonitor_initial_rules(monitors[i], &rules); } + struct ovs_list replies; ofpmp_init(&replies, oh); ofmonitor_compose_refresh_updates(&rules, &replies); ovs_mutex_unlock(&ofproto_mutex); @@ -5663,7 +5660,7 @@ handle_flow_monitor_request(struct ofconn *ofconn, const struct ofp_header *oh) return 0; error: - for (i = 0; i < n_monitors; i++) { + for (size_t i = 0; i < n_monitors; i++) { ofmonitor_destroy(monitors[i]); } free(monitors); diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c index 96d6c89d9..656b8b3b8 100644 --- a/utilities/ovs-ofctl.c +++ b/utilities/ovs-ofctl.c @@ -1987,18 +1987,16 @@ fetch_table_desc(struct vconn *vconn, struct ofputil_table_mod *tm, recv_xid = ((struct ofp_header *) reply->data)->xid; if (send_xid == recv_xid) { struct ofp_header *oh = reply->data; - enum ofptype type; - struct ofpbuf b; - uint16_t flags; + struct ofpbuf b = ofpbuf_const_initializer(oh, ntohs(oh->length)); - ofpbuf_use_const(&b, oh, ntohs(oh->length)); + enum ofptype type; if (ofptype_pull(&type, &b) || type != OFPTYPE_TABLE_DESC_REPLY) { ovs_fatal(0, "received bad reply: %s", ofp_to_string(reply->data, reply->size, verbosity + 1)); } - flags = ofpmp_flags(oh); + uint16_t flags = ofpmp_flags(oh); done = !(flags & OFPSF_REPLY_MORE); if (found) { /* We've already found the table desc consisting of current -- 2.20.1