Looks good.
--Justin
On Jun 29, 2012, at 10:40 PM, Ben Pfaff wrote:
> This seems like sensible return value semantics to me, even though the new
> operation is also available through rule->pending.
>
> This is a code cleanup only that should not affect behavior.
>
> Signed-off-by: Ben Pfaff <b...@nicira.com>
> ---
> ofproto/ofproto.c | 27 ++++++++++++++++++---------
> 1 files changed, 18 insertions(+), 9 deletions(-)
>
> diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
> index 5221318..0e3009a 100644
> --- a/ofproto/ofproto.c
> +++ b/ofproto/ofproto.c
> @@ -123,8 +123,9 @@ struct ofoperation {
> ovs_be64 flow_cookie; /* Rule's old flow cookie. */
> };
>
> -static void ofoperation_create(struct ofopgroup *, struct rule *,
> - enum ofoperation_type);
> +static struct ofoperation *ofoperation_create(struct ofopgroup *,
> + struct rule *,
> + enum ofoperation_type);
> static void ofoperation_destroy(struct ofoperation *);
>
> /* oftable. */
> @@ -2851,6 +2852,7 @@ add_flow(struct ofproto *ofproto, struct ofconn *ofconn,
> } else if (victim && victim->pending) {
> error = OFPROTO_POSTPONE;
> } else {
> + struct ofoperation *op;
> struct rule *evict;
>
> if (classifier_count(&table->cls) > table->max_flows) {
> @@ -2873,8 +2875,8 @@ add_flow(struct ofproto *ofproto, struct ofconn *ofconn,
> }
>
> group = ofopgroup_create(ofproto, ofconn, request, fm->buffer_id);
> - ofoperation_create(group, rule, OFOPERATION_ADD);
> - rule->pending->victim = victim;
> + op = ofoperation_create(group, rule, OFOPERATION_ADD);
> + op->victim = victim;
>
> error = ofproto->ofproto_class->rule_construct(rule);
> if (error) {
> @@ -2924,9 +2926,11 @@ modify_flows__(struct ofproto *ofproto, struct ofconn
> *ofconn,
>
> if (!ofputil_actions_equal(fm->actions, fm->n_actions,
> rule->actions, rule->n_actions)) {
> - ofoperation_create(group, rule, OFOPERATION_MODIFY);
> - rule->pending->actions = rule->actions;
> - rule->pending->n_actions = rule->n_actions;
> + struct ofoperation *op;
> +
> + op = ofoperation_create(group, rule, OFOPERATION_MODIFY);
> + op->actions = rule->actions;
> + op->n_actions = rule->n_actions;
> rule->actions = ofputil_actions_clone(fm->actions, fm->n_actions);
> rule->n_actions = fm->n_actions;
> ofproto->ofproto_class->rule_modify_actions(rule);
> @@ -3580,8 +3584,11 @@ ofopgroup_destroy(struct ofopgroup *group)
> }
>
> /* Initiates a new operation on 'rule', of the specified 'type', within
> - * 'group'. Prior to calling, 'rule' must not have any pending operation. */
> -static void
> + * 'group'. Prior to calling, 'rule' must not have any pending operation.
> + *
> + * Returns the newly created ofoperation (which is also available as
> + * rule->pending). */
> +static struct ofoperation *
> ofoperation_create(struct ofopgroup *group, struct rule *rule,
> enum ofoperation_type type)
> {
> @@ -3601,6 +3608,8 @@ ofoperation_create(struct ofopgroup *group, struct rule
> *rule,
> hmap_insert(&ofproto->deletions, &op->hmap_node,
> cls_rule_hash(&rule->cr, rule->table_id));
> }
> +
> + return op;
> }
>
> static void
> --
> 1.7.2.5
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
