[ovs-dev] [overload 1/7] ofproto: Avoid wasting memory malloc()'ing empty action sets for subrules.
Justin Pettit
jpettit at nicira.com
Fri Oct 1 22:19:24 UTC 2010
This looks like a good improvement. I'm a bit concerned about code that may not be defensively written to protect against "rule->actions" being NULL. For example the following call in flow_stats_ds_cb():
ofp_print_actions(results, &rule->actions->header, act_len);
This isn't currently a problem due to a check earlier in the function for whether it's a subrule. However, the check isn't about whether "rule->n_actions" is 0, so it seems like it wouldn't be hard to accidentally introduce a segfault.
--Justin
On Sep 30, 2010, at 5:17 PM, Ben Pfaff wrote:
> GNU libc treats malloc(0) as malloc(1). Subrules always have an n_actions
> of 0, so this code was wasting time and memory for subrules. This commit
> stops doing that.
> ---
> ofproto/ofproto.c | 6 ++++--
> 1 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
> index e571bd4..ee05fdd 100644
> --- a/ofproto/ofproto.c
> +++ b/ofproto/ofproto.c
> @@ -1863,8 +1863,10 @@ rule_create(struct ofproto *ofproto, struct rule *super,
> } else {
> list_init(&rule->list);
> }
> - rule->n_actions = n_actions;
> - rule->actions = xmemdup(actions, n_actions * sizeof *actions);
> + if (n_actions > 0) {
> + rule->n_actions = n_actions;
> + rule->actions = xmemdup(actions, n_actions * sizeof *actions);
> + }
> netflow_flow_clear(&rule->nf_flow);
> netflow_flow_update_time(ofproto->netflow, &rule->nf_flow, rule->created);
>
> --
> 1.7.1
>
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev_openvswitch.org
More information about the dev
mailing list