| Message ID | 20181114134145.15978-1-i.maximets@samsung.com |
|---|---|
| State | Accepted |
| Delegated to: | Ian Stokes |
| Headers | show |
| Series | [ovs-dev,dpdk-latest] netdev-dpdk: Fix returning the field of malloced struct. | expand |
On 11/14/2018 01:41 PM, Ilya Maximets wrote: > There is no sense returning the field because it never used. > Function returns the pointer just to allow the caller to free it. > Returning the pointer to the actual structure simplifies the code > allowing not to explain why we're freeing the container of. > Acked-by: Kevin Traynor <ktraynor@redhat.com> > Signed-off-by: Ilya Maximets <i.maximets@samsung.com> > --- > lib/netdev-dpdk.c | 15 ++++----------- > 1 file changed, 4 insertions(+), 11 deletions(-) > > diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c > index 1480bf8d1..3e84a7fdf 100644 > --- a/lib/netdev-dpdk.c > +++ b/lib/netdev-dpdk.c > @@ -4412,7 +4412,7 @@ struct action_rss_data { > uint16_t queue[0]; > }; > > -static struct rte_flow_action_rss * > +static struct action_rss_data * > add_flow_rss_action(struct flow_actions *actions, > struct netdev *netdev) { > int i; > @@ -4439,7 +4439,7 @@ add_flow_rss_action(struct flow_actions *actions, > > add_flow_action(actions, RTE_FLOW_ACTION_TYPE_RSS, &rss_data->conf); > > - return &rss_data->conf; > + return rss_data; > } > > static int > @@ -4649,7 +4649,7 @@ end_proto_check: > add_flow_pattern(&patterns, RTE_FLOW_ITEM_TYPE_END, NULL, NULL); > > struct rte_flow_action_mark mark; > - struct rte_flow_action_rss *rss; > + struct action_rss_data *rss; > > mark.id = info->flow_mark; > add_flow_action(&actions, RTE_FLOW_ACTION_TYPE_MARK, &mark); > @@ -4664,14 +4664,7 @@ end_proto_check: > > ovs_mutex_unlock(&dev->mutex); > > - /* > - * 'rss' points to a memory (struct rte_flow_action_rss) that is contained > - * in a bigger memory (struct action_rss_data) that was allocated in > - * function add_flow_rss_actions(). The bigger memory holds additional > - * space for the RSS queues. Given the 'rss' pointer we are freeing the > - * bigger memory by using the CONTAINER_OF() macro. > - */ > - free(CONTAINER_OF(rss, struct action_rss_data, conf)); > + free(rss); > if (!flow) { > VLOG_ERR("%s: rte flow creat error: %u : message : %s\n", > netdev_get_name(netdev), error.type, error.message); >
> On 11/14/2018 01:41 PM, Ilya Maximets wrote: > > There is no sense returning the field because it never used. > > Function returns the pointer just to allow the caller to free it. > > Returning the pointer to the actual structure simplifies the code > > allowing not to explain why we're freeing the container of. > > > > Acked-by: Kevin Traynor <ktraynor@redhat.com> > Thanks all, I've applied this to dpdk-latest and dpdk-hwol. Ian
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index 1480bf8d1..3e84a7fdf 100644 --- a/lib/netdev-dpdk.c +++ b/lib/netdev-dpdk.c @@ -4412,7 +4412,7 @@ struct action_rss_data { uint16_t queue[0]; }; -static struct rte_flow_action_rss * +static struct action_rss_data * add_flow_rss_action(struct flow_actions *actions, struct netdev *netdev) { int i; @@ -4439,7 +4439,7 @@ add_flow_rss_action(struct flow_actions *actions, add_flow_action(actions, RTE_FLOW_ACTION_TYPE_RSS, &rss_data->conf); - return &rss_data->conf; + return rss_data; } static int @@ -4649,7 +4649,7 @@ end_proto_check: add_flow_pattern(&patterns, RTE_FLOW_ITEM_TYPE_END, NULL, NULL); struct rte_flow_action_mark mark; - struct rte_flow_action_rss *rss; + struct action_rss_data *rss; mark.id = info->flow_mark; add_flow_action(&actions, RTE_FLOW_ACTION_TYPE_MARK, &mark); @@ -4664,14 +4664,7 @@ end_proto_check: ovs_mutex_unlock(&dev->mutex); - /* - * 'rss' points to a memory (struct rte_flow_action_rss) that is contained - * in a bigger memory (struct action_rss_data) that was allocated in - * function add_flow_rss_actions(). The bigger memory holds additional - * space for the RSS queues. Given the 'rss' pointer we are freeing the - * bigger memory by using the CONTAINER_OF() macro. - */ - free(CONTAINER_OF(rss, struct action_rss_data, conf)); + free(rss); if (!flow) { VLOG_ERR("%s: rte flow creat error: %u : message : %s\n", netdev_get_name(netdev), error.type, error.message);
There is no sense returning the field because it never used. Function returns the pointer just to allow the caller to free it. Returning the pointer to the actual structure simplifies the code allowing not to explain why we're freeing the container of. Signed-off-by: Ilya Maximets <i.maximets@samsung.com> --- lib/netdev-dpdk.c | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-)