[dpdk-dev] [PATCH v3] examples/qos_meter: fix unchecked return value
> -Original Message-
> From: Dumitrescu, Cristian
> Sent: Thursday, May 12, 2016 10:41 AM
> To: Mrozowicz, SlawomirX
> Cc: dev at dpdk.org; Singh, Jasvinder
> Subject: RE: [PATCH v3] examples/qos_meter: fix unchecked return value
>
>
>
> > -Original Message-
> > From: Mrozowicz, SlawomirX
> > Sent: Thursday, May 12, 2016 8:06 AM
> > To: Dumitrescu, Cristian
> > Cc: dev at dpdk.org; Singh, Jasvinder
> > Subject: RE: [PATCH v3] examples/qos_meter: fix unchecked return value
> >
> >
> >
> > >-Original Message-
> > >From: Dumitrescu, Cristian
> > >Sent: Wednesday, May 11, 2016 7:57 PM
> > >To: Mrozowicz, SlawomirX
> > >Cc: dev at dpdk.org; Singh, Jasvinder
> > >Subject: RE: [PATCH v3] examples/qos_meter: fix unchecked return value
> > >
> > >
> > >
> > >> -Original Message-
> > >> From: Mrozowicz, SlawomirX
> > >> Sent: Wednesday, May 11, 2016 10:15 AM
> > >> To: Dumitrescu, Cristian
> > >> Cc: dev at dpdk.org; Singh, Jasvinder
> > >> Subject: RE: [PATCH v3] examples/qos_meter: fix unchecked return
> value
> > >>
> > >>
> > >>
> > >> >-Original Message-
> > >> >From: Dumitrescu, Cristian
> > >> >Sent: Tuesday, May 10, 2016 7:42 PM
> > >> >To: Mrozowicz, SlawomirX
> > >> >Cc: dev at dpdk.org; Singh, Jasvinder
> > >> >Subject: RE: [PATCH v3] examples/qos_meter: fix unchecked return
> > >> >value
> > >> >
> > >> >
> > >> >
> > >> >> -Original Message-
> > >> >> From: Mrozowicz, SlawomirX
> > >> >> Sent: Monday, May 9, 2016 9:38 AM
> > >> >> To: Dumitrescu, Cristian
> > >> >> Cc: dev at dpdk.org; Singh, Jasvinder ;
> > >> >> Mrozowicz, SlawomirX
> > >> >> Subject: [PATCH v3] examples/qos_meter: fix unchecked return
> value
> > >> >>
> > >> >> Fix issue reported by Coverity.
> > >> >>
> > >> >> Coverity ID 30693: Unchecked return value
> > >> >> check_return: Calling rte_meter_srtcm_config without checking
> > >> >> return value.
> > >> >>
> > >> >> Fixes: e6541fdec8b2 ("meter: initial import")
> > >> >>
> > >> >> Signed-off-by: Slawomir Mrozowicz
> >
> > >> >> ---
> > >> >> examples/qos_meter/main.c | 15 ++-
> > >> >> examples/qos_meter/main.h | 2 +-
> > >> >> 2 files changed, 11 insertions(+), 6 deletions(-)
> > >> >>
> > >> >> diff --git a/examples/qos_meter/main.c
> > b/examples/qos_meter/main.c
> > >> >> index b968b00..7c69606 100644
> > >> >> --- a/examples/qos_meter/main.c
> > >> >> +++ b/examples/qos_meter/main.c
> > >> >> @@ -133,14 +133,17 @@ struct rte_meter_trtcm_params
> > >> >app_trtcm_params[]
> > >> >> = {
> > >> >>
> > >> >> FLOW_METER app_flows[APP_FLOWS_MAX];
> > >> >>
> > >> >> -static void
> > >> >> +static int
> > >> >> app_configure_flow_table(void)
> > >> >> {
> > >> >> uint32_t i, j;
> > >> >> + int ret = 0;
> > >> >>
> > >> >> - for (i = 0, j = 0; i < APP_FLOWS_MAX; i ++, j = (j + 1) %
> > >> >> RTE_DIM(PARAMS)){
> > >> >> - FUNC_CONFIG(_flows[i], [j]);
> > >> >> - }
> > >> >> + for (i = 0, j = 0; i < APP_FLOWS_MAX && ret == 0;
> > >> >> + i ++, j = (j + 1) % RTE_DIM(PARAMS))
> > >> >> + ret = FUNC_CONFIG(_flows[i], [j]);
> > >> >> +
> > >> >> + return ret;
> > >> >> }
> > >> >
> > >> >This is only returns the configuration status for the last flow and
> > >> >leaves undetected an error for any other flow. Why not check the
> > >> >status for each flow and return an error on first occurrence?
> > >> >For (...){ret = FUNC_CONFIG(...); if (ret) return ret;}
> > >> >
> > >>
> > >> This code check status of the function FUNC_CONFIG for each flow and
> > >> return an error on first occurrence. Rest of functions FUNC_CONFIG are
> > >> not called. See terminate condition of the loop.
> > >>
> > >
> > >Where is the status of FUNC_CONFIG checked exactly? I cannot see any
> > check
> > >in your code. I can only see returning the status code for the last call of
> this
> > >function in the for loop. I was expecting a check such as: if (ret) return
> > >ret.
> > >
> >
> > Look at the loop terminate conditions:
> > i < APP_FLOWS_MAX && ret == 0
> > Program terminate the loop if the ret variable is differ then zero.
> > It means that program terminate if the last status of FUNC_CONFIG is an
> > error.
>
> Yes, you're right, my bad, sorry.
>
Actually, although the logic is correct, personally I find this code very
cryptic / hard to read and more difficult to extend later if need be, can we
please make it a bit more readable:
for (i = 0, j = 0; i < APP_FLOWS_MAX; i ++, j = (j + 1) % RTE_DIM(PARAMS)) {
ret = FUNC_CONFIG(_flows[i], [j]);
if (ret)
return ret;
}
return ret;
Thanks, Slawomir!
> >
> > >> >>
> > >> >> static inline void
> > >> >> @@ -381,7 +384,9 @@ main(int argc, char **argv)
> > >> >> rte_eth_promiscuous_enable(port_tx);
> > >> >>
> > >> >> /* App configuration */
> > >> >> - app_configure_flow_table();
> > >> >> + ret = app_configure_flow_table();
> > >> >> + if (ret < 0)
>
[dpdk-dev] [PATCH v3] examples/qos_meter: fix unchecked return value
> -Original Message-
> From: Mrozowicz, SlawomirX
> Sent: Thursday, May 12, 2016 8:06 AM
> To: Dumitrescu, Cristian
> Cc: dev at dpdk.org; Singh, Jasvinder
> Subject: RE: [PATCH v3] examples/qos_meter: fix unchecked return value
>
>
>
> >-Original Message-
> >From: Dumitrescu, Cristian
> >Sent: Wednesday, May 11, 2016 7:57 PM
> >To: Mrozowicz, SlawomirX
> >Cc: dev at dpdk.org; Singh, Jasvinder
> >Subject: RE: [PATCH v3] examples/qos_meter: fix unchecked return value
> >
> >
> >
> >> -Original Message-
> >> From: Mrozowicz, SlawomirX
> >> Sent: Wednesday, May 11, 2016 10:15 AM
> >> To: Dumitrescu, Cristian
> >> Cc: dev at dpdk.org; Singh, Jasvinder
> >> Subject: RE: [PATCH v3] examples/qos_meter: fix unchecked return value
> >>
> >>
> >>
> >> >-Original Message-
> >> >From: Dumitrescu, Cristian
> >> >Sent: Tuesday, May 10, 2016 7:42 PM
> >> >To: Mrozowicz, SlawomirX
> >> >Cc: dev at dpdk.org; Singh, Jasvinder
> >> >Subject: RE: [PATCH v3] examples/qos_meter: fix unchecked return
> >> >value
> >> >
> >> >
> >> >
> >> >> -Original Message-
> >> >> From: Mrozowicz, SlawomirX
> >> >> Sent: Monday, May 9, 2016 9:38 AM
> >> >> To: Dumitrescu, Cristian
> >> >> Cc: dev at dpdk.org; Singh, Jasvinder ;
> >> >> Mrozowicz, SlawomirX
> >> >> Subject: [PATCH v3] examples/qos_meter: fix unchecked return value
> >> >>
> >> >> Fix issue reported by Coverity.
> >> >>
> >> >> Coverity ID 30693: Unchecked return value
> >> >> check_return: Calling rte_meter_srtcm_config without checking
> >> >> return value.
> >> >>
> >> >> Fixes: e6541fdec8b2 ("meter: initial import")
> >> >>
> >> >> Signed-off-by: Slawomir Mrozowicz
>
> >> >> ---
> >> >> examples/qos_meter/main.c | 15 ++-
> >> >> examples/qos_meter/main.h | 2 +-
> >> >> 2 files changed, 11 insertions(+), 6 deletions(-)
> >> >>
> >> >> diff --git a/examples/qos_meter/main.c
> b/examples/qos_meter/main.c
> >> >> index b968b00..7c69606 100644
> >> >> --- a/examples/qos_meter/main.c
> >> >> +++ b/examples/qos_meter/main.c
> >> >> @@ -133,14 +133,17 @@ struct rte_meter_trtcm_params
> >> >app_trtcm_params[]
> >> >> = {
> >> >>
> >> >> FLOW_METER app_flows[APP_FLOWS_MAX];
> >> >>
> >> >> -static void
> >> >> +static int
> >> >> app_configure_flow_table(void)
> >> >> {
> >> >> uint32_t i, j;
> >> >> + int ret = 0;
> >> >>
> >> >> - for (i = 0, j = 0; i < APP_FLOWS_MAX; i ++, j = (j + 1) %
> >> >> RTE_DIM(PARAMS)){
> >> >> - FUNC_CONFIG(_flows[i], [j]);
> >> >> - }
> >> >> + for (i = 0, j = 0; i < APP_FLOWS_MAX && ret == 0;
> >> >> + i ++, j = (j + 1) % RTE_DIM(PARAMS))
> >> >> + ret = FUNC_CONFIG(_flows[i], [j]);
> >> >> +
> >> >> + return ret;
> >> >> }
> >> >
> >> >This is only returns the configuration status for the last flow and
> >> >leaves undetected an error for any other flow. Why not check the
> >> >status for each flow and return an error on first occurrence?
> >> >For (...){ret = FUNC_CONFIG(...); if (ret) return ret;}
> >> >
> >>
> >> This code check status of the function FUNC_CONFIG for each flow and
> >> return an error on first occurrence. Rest of functions FUNC_CONFIG are
> >> not called. See terminate condition of the loop.
> >>
> >
> >Where is the status of FUNC_CONFIG checked exactly? I cannot see any
> check
> >in your code. I can only see returning the status code for the last call of
> >this
> >function in the for loop. I was expecting a check such as: if (ret) return
> >ret.
> >
>
> Look at the loop terminate conditions:
> i < APP_FLOWS_MAX && ret == 0
> Program terminate the loop if the ret variable is differ then zero.
> It means that program terminate if the last status of FUNC_CONFIG is an
> error.
Yes, you're right, my bad, sorry.
>
> >> >>
> >> >> static inline void
> >> >> @@ -381,7 +384,9 @@ main(int argc, char **argv)
> >> >> rte_eth_promiscuous_enable(port_tx);
> >> >>
> >> >> /* App configuration */
> >> >> - app_configure_flow_table();
> >> >> + ret = app_configure_flow_table();
> >> >> + if (ret < 0)
> >> >> + rte_exit(EXIT_FAILURE, "Invalid configure flow
> >> >> table\n");
> >> >>
> >> >> /* Launch per-lcore init on every lcore */
> >> >> rte_eal_mp_remote_launch(main_loop, NULL, CALL_MASTER); diff -
> >> -
> >> >git
> >> >> a/examples/qos_meter/main.h b/examples/qos_meter/main.h index
> >> >> 530bf69..54867dc 100644
> >> >> --- a/examples/qos_meter/main.h
> >> >> +++ b/examples/qos_meter/main.h
> >> >> @@ -51,7 +51,7 @@ enum policer_action
> >> >> policer_table[e_RTE_METER_COLORS][e_RTE_METER_COLORS] = #if
> >> >APP_MODE
> >> >> == APP_MODE_FWD
> >> >>
> >> >> #define FUNC_METER(a,b,c,d) color, flow_id=flow_id,
> >> >> pkt_len=pkt_len, time=time -#define FUNC_CONFIG(a,b)
> >> >> +#define FUNC_CONFIG(a, b) 0
> >> >> #define PARAMS app_srtcm_params
> >> >> #define FLOW_METER int
> >> >>
> >> >> --
> >> >>
[dpdk-dev] [PATCH v3] examples/qos_meter: fix unchecked return value
>-Original Message-
>From: Dumitrescu, Cristian
>Sent: Wednesday, May 11, 2016 7:57 PM
>To: Mrozowicz, SlawomirX
>Cc: dev at dpdk.org; Singh, Jasvinder
>Subject: RE: [PATCH v3] examples/qos_meter: fix unchecked return value
>
>
>
>> -Original Message-
>> From: Mrozowicz, SlawomirX
>> Sent: Wednesday, May 11, 2016 10:15 AM
>> To: Dumitrescu, Cristian
>> Cc: dev at dpdk.org; Singh, Jasvinder
>> Subject: RE: [PATCH v3] examples/qos_meter: fix unchecked return value
>>
>>
>>
>> >-Original Message-
>> >From: Dumitrescu, Cristian
>> >Sent: Tuesday, May 10, 2016 7:42 PM
>> >To: Mrozowicz, SlawomirX
>> >Cc: dev at dpdk.org; Singh, Jasvinder
>> >Subject: RE: [PATCH v3] examples/qos_meter: fix unchecked return
>> >value
>> >
>> >
>> >
>> >> -Original Message-
>> >> From: Mrozowicz, SlawomirX
>> >> Sent: Monday, May 9, 2016 9:38 AM
>> >> To: Dumitrescu, Cristian
>> >> Cc: dev at dpdk.org; Singh, Jasvinder ;
>> >> Mrozowicz, SlawomirX
>> >> Subject: [PATCH v3] examples/qos_meter: fix unchecked return value
>> >>
>> >> Fix issue reported by Coverity.
>> >>
>> >> Coverity ID 30693: Unchecked return value
>> >> check_return: Calling rte_meter_srtcm_config without checking
>> >> return value.
>> >>
>> >> Fixes: e6541fdec8b2 ("meter: initial import")
>> >>
>> >> Signed-off-by: Slawomir Mrozowicz
>> >> ---
>> >> examples/qos_meter/main.c | 15 ++-
>> >> examples/qos_meter/main.h | 2 +-
>> >> 2 files changed, 11 insertions(+), 6 deletions(-)
>> >>
>> >> diff --git a/examples/qos_meter/main.c b/examples/qos_meter/main.c
>> >> index b968b00..7c69606 100644
>> >> --- a/examples/qos_meter/main.c
>> >> +++ b/examples/qos_meter/main.c
>> >> @@ -133,14 +133,17 @@ struct rte_meter_trtcm_params
>> >app_trtcm_params[]
>> >> = {
>> >>
>> >> FLOW_METER app_flows[APP_FLOWS_MAX];
>> >>
>> >> -static void
>> >> +static int
>> >> app_configure_flow_table(void)
>> >> {
>> >> uint32_t i, j;
>> >> + int ret = 0;
>> >>
>> >> - for (i = 0, j = 0; i < APP_FLOWS_MAX; i ++, j = (j + 1) %
>> >> RTE_DIM(PARAMS)){
>> >> - FUNC_CONFIG(_flows[i], [j]);
>> >> - }
>> >> + for (i = 0, j = 0; i < APP_FLOWS_MAX && ret == 0;
>> >> + i ++, j = (j + 1) % RTE_DIM(PARAMS))
>> >> + ret = FUNC_CONFIG(_flows[i], [j]);
>> >> +
>> >> + return ret;
>> >> }
>> >
>> >This is only returns the configuration status for the last flow and
>> >leaves undetected an error for any other flow. Why not check the
>> >status for each flow and return an error on first occurrence?
>> >For (...){ret = FUNC_CONFIG(...); if (ret) return ret;}
>> >
>>
>> This code check status of the function FUNC_CONFIG for each flow and
>> return an error on first occurrence. Rest of functions FUNC_CONFIG are
>> not called. See terminate condition of the loop.
>>
>
>Where is the status of FUNC_CONFIG checked exactly? I cannot see any check
>in your code. I can only see returning the status code for the last call of
>this
>function in the for loop. I was expecting a check such as: if (ret) return ret.
>
Look at the loop terminate conditions:
i < APP_FLOWS_MAX && ret == 0
Program terminate the loop if the ret variable is differ then zero.
It means that program terminate if the last status of FUNC_CONFIG is an error.
>> >>
>> >> static inline void
>> >> @@ -381,7 +384,9 @@ main(int argc, char **argv)
>> >> rte_eth_promiscuous_enable(port_tx);
>> >>
>> >> /* App configuration */
>> >> - app_configure_flow_table();
>> >> + ret = app_configure_flow_table();
>> >> + if (ret < 0)
>> >> + rte_exit(EXIT_FAILURE, "Invalid configure flow table\n");
>> >>
>> >> /* Launch per-lcore init on every lcore */
>> >> rte_eal_mp_remote_launch(main_loop, NULL, CALL_MASTER); diff -
>> -
>> >git
>> >> a/examples/qos_meter/main.h b/examples/qos_meter/main.h index
>> >> 530bf69..54867dc 100644
>> >> --- a/examples/qos_meter/main.h
>> >> +++ b/examples/qos_meter/main.h
>> >> @@ -51,7 +51,7 @@ enum policer_action
>> >> policer_table[e_RTE_METER_COLORS][e_RTE_METER_COLORS] = #if
>> >APP_MODE
>> >> == APP_MODE_FWD
>> >>
>> >> #define FUNC_METER(a,b,c,d) color, flow_id=flow_id,
>> >> pkt_len=pkt_len, time=time -#define FUNC_CONFIG(a,b)
>> >> +#define FUNC_CONFIG(a, b) 0
>> >> #define PARAMS app_srtcm_params
>> >> #define FLOW_METER int
>> >>
>> >> --
>> >> 1.9.1
[dpdk-dev] [PATCH v3] examples/qos_meter: fix unchecked return value
> -Original Message-
> From: Mrozowicz, SlawomirX
> Sent: Wednesday, May 11, 2016 10:15 AM
> To: Dumitrescu, Cristian
> Cc: dev at dpdk.org; Singh, Jasvinder
> Subject: RE: [PATCH v3] examples/qos_meter: fix unchecked return value
>
>
>
> >-Original Message-
> >From: Dumitrescu, Cristian
> >Sent: Tuesday, May 10, 2016 7:42 PM
> >To: Mrozowicz, SlawomirX
> >Cc: dev at dpdk.org; Singh, Jasvinder
> >Subject: RE: [PATCH v3] examples/qos_meter: fix unchecked return value
> >
> >
> >
> >> -Original Message-
> >> From: Mrozowicz, SlawomirX
> >> Sent: Monday, May 9, 2016 9:38 AM
> >> To: Dumitrescu, Cristian
> >> Cc: dev at dpdk.org; Singh, Jasvinder ;
> >> Mrozowicz, SlawomirX
> >> Subject: [PATCH v3] examples/qos_meter: fix unchecked return value
> >>
> >> Fix issue reported by Coverity.
> >>
> >> Coverity ID 30693: Unchecked return value
> >> check_return: Calling rte_meter_srtcm_config without checking return
> >> value.
> >>
> >> Fixes: e6541fdec8b2 ("meter: initial import")
> >>
> >> Signed-off-by: Slawomir Mrozowicz
> >> ---
> >> examples/qos_meter/main.c | 15 ++-
> >> examples/qos_meter/main.h | 2 +-
> >> 2 files changed, 11 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/examples/qos_meter/main.c b/examples/qos_meter/main.c
> >> index b968b00..7c69606 100644
> >> --- a/examples/qos_meter/main.c
> >> +++ b/examples/qos_meter/main.c
> >> @@ -133,14 +133,17 @@ struct rte_meter_trtcm_params
> >app_trtcm_params[]
> >> = {
> >>
> >> FLOW_METER app_flows[APP_FLOWS_MAX];
> >>
> >> -static void
> >> +static int
> >> app_configure_flow_table(void)
> >> {
> >>uint32_t i, j;
> >> + int ret = 0;
> >>
> >> - for (i = 0, j = 0; i < APP_FLOWS_MAX; i ++, j = (j + 1) %
> >> RTE_DIM(PARAMS)){
> >> - FUNC_CONFIG(_flows[i], [j]);
> >> - }
> >> + for (i = 0, j = 0; i < APP_FLOWS_MAX && ret == 0;
> >> + i ++, j = (j + 1) % RTE_DIM(PARAMS))
> >> + ret = FUNC_CONFIG(_flows[i], [j]);
> >> +
> >> + return ret;
> >> }
> >
> >This is only returns the configuration status for the last flow and leaves
> >undetected an error for any other flow. Why not check the status for each
> >flow and return an error on first occurrence?
> >For (...){ret = FUNC_CONFIG(...); if (ret) return ret;}
> >
>
> This code check status of the function FUNC_CONFIG for each flow and
> return an error on first occurrence. Rest of functions FUNC_CONFIG are not
> called. See terminate condition of the loop.
>
Where is the status of FUNC_CONFIG checked exactly? I cannot see any check in
your code. I can only see returning the status code for the last call of this
function in the for loop. I was expecting a check such as: if (ret) return ret.
> >>
> >> static inline void
> >> @@ -381,7 +384,9 @@ main(int argc, char **argv)
> >>rte_eth_promiscuous_enable(port_tx);
> >>
> >>/* App configuration */
> >> - app_configure_flow_table();
> >> + ret = app_configure_flow_table();
> >> + if (ret < 0)
> >> + rte_exit(EXIT_FAILURE, "Invalid configure flow table\n");
> >>
> >>/* Launch per-lcore init on every lcore */
> >>rte_eal_mp_remote_launch(main_loop, NULL, CALL_MASTER); diff -
> -
> >git
> >> a/examples/qos_meter/main.h b/examples/qos_meter/main.h index
> >> 530bf69..54867dc 100644
> >> --- a/examples/qos_meter/main.h
> >> +++ b/examples/qos_meter/main.h
> >> @@ -51,7 +51,7 @@ enum policer_action
> >> policer_table[e_RTE_METER_COLORS][e_RTE_METER_COLORS] = #if
> >APP_MODE
> >> == APP_MODE_FWD
> >>
> >> #define FUNC_METER(a,b,c,d) color, flow_id=flow_id, pkt_len=pkt_len,
> >> time=time -#define FUNC_CONFIG(a,b)
> >> +#define FUNC_CONFIG(a, b) 0
> >> #define PARAMSapp_srtcm_params
> >> #define FLOW_METER int
> >>
> >> --
> >> 1.9.1
[dpdk-dev] [PATCH v3] examples/qos_meter: fix unchecked return value
>-Original Message-
>From: Dumitrescu, Cristian
>Sent: Tuesday, May 10, 2016 7:42 PM
>To: Mrozowicz, SlawomirX
>Cc: dev at dpdk.org; Singh, Jasvinder
>Subject: RE: [PATCH v3] examples/qos_meter: fix unchecked return value
>
>
>
>> -Original Message-
>> From: Mrozowicz, SlawomirX
>> Sent: Monday, May 9, 2016 9:38 AM
>> To: Dumitrescu, Cristian
>> Cc: dev at dpdk.org; Singh, Jasvinder ;
>> Mrozowicz, SlawomirX
>> Subject: [PATCH v3] examples/qos_meter: fix unchecked return value
>>
>> Fix issue reported by Coverity.
>>
>> Coverity ID 30693: Unchecked return value
>> check_return: Calling rte_meter_srtcm_config without checking return
>> value.
>>
>> Fixes: e6541fdec8b2 ("meter: initial import")
>>
>> Signed-off-by: Slawomir Mrozowicz
>> ---
>> examples/qos_meter/main.c | 15 ++-
>> examples/qos_meter/main.h | 2 +-
>> 2 files changed, 11 insertions(+), 6 deletions(-)
>>
>> diff --git a/examples/qos_meter/main.c b/examples/qos_meter/main.c
>> index b968b00..7c69606 100644
>> --- a/examples/qos_meter/main.c
>> +++ b/examples/qos_meter/main.c
>> @@ -133,14 +133,17 @@ struct rte_meter_trtcm_params
>app_trtcm_params[]
>> = {
>>
>> FLOW_METER app_flows[APP_FLOWS_MAX];
>>
>> -static void
>> +static int
>> app_configure_flow_table(void)
>> {
>> uint32_t i, j;
>> +int ret = 0;
>>
>> -for (i = 0, j = 0; i < APP_FLOWS_MAX; i ++, j = (j + 1) %
>> RTE_DIM(PARAMS)){
>> -FUNC_CONFIG(_flows[i], [j]);
>> -}
>> +for (i = 0, j = 0; i < APP_FLOWS_MAX && ret == 0;
>> +i ++, j = (j + 1) % RTE_DIM(PARAMS))
>> +ret = FUNC_CONFIG(_flows[i], [j]);
>> +
>> +return ret;
>> }
>
>This is only returns the configuration status for the last flow and leaves
>undetected an error for any other flow. Why not check the status for each
>flow and return an error on first occurrence?
>For (...){ret = FUNC_CONFIG(...); if (ret) return ret;}
>
This code check status of the function FUNC_CONFIG for each flow and return an
error on first occurrence. Rest of functions FUNC_CONFIG are not called. See
terminate condition of the loop.
>>
>> static inline void
>> @@ -381,7 +384,9 @@ main(int argc, char **argv)
>> rte_eth_promiscuous_enable(port_tx);
>>
>> /* App configuration */
>> -app_configure_flow_table();
>> +ret = app_configure_flow_table();
>> +if (ret < 0)
>> +rte_exit(EXIT_FAILURE, "Invalid configure flow table\n");
>>
>> /* Launch per-lcore init on every lcore */
>> rte_eal_mp_remote_launch(main_loop, NULL, CALL_MASTER); diff --
>git
>> a/examples/qos_meter/main.h b/examples/qos_meter/main.h index
>> 530bf69..54867dc 100644
>> --- a/examples/qos_meter/main.h
>> +++ b/examples/qos_meter/main.h
>> @@ -51,7 +51,7 @@ enum policer_action
>> policer_table[e_RTE_METER_COLORS][e_RTE_METER_COLORS] = #if
>APP_MODE
>> == APP_MODE_FWD
>>
>> #define FUNC_METER(a,b,c,d) color, flow_id=flow_id, pkt_len=pkt_len,
>> time=time -#define FUNC_CONFIG(a,b)
>> +#define FUNC_CONFIG(a, b) 0
>> #define PARAMS app_srtcm_params
>> #define FLOW_METER int
>>
>> --
>> 1.9.1
[dpdk-dev] [PATCH v3] examples/qos_meter: fix unchecked return value
> -Original Message-
> From: Mrozowicz, SlawomirX
> Sent: Monday, May 9, 2016 9:38 AM
> To: Dumitrescu, Cristian
> Cc: dev at dpdk.org; Singh, Jasvinder ;
> Mrozowicz, SlawomirX
> Subject: [PATCH v3] examples/qos_meter: fix unchecked return value
>
> Fix issue reported by Coverity.
>
> Coverity ID 30693: Unchecked return value
> check_return: Calling rte_meter_srtcm_config without checking return
> value.
>
> Fixes: e6541fdec8b2 ("meter: initial import")
>
> Signed-off-by: Slawomir Mrozowicz
> ---
> examples/qos_meter/main.c | 15 ++-
> examples/qos_meter/main.h | 2 +-
> 2 files changed, 11 insertions(+), 6 deletions(-)
>
> diff --git a/examples/qos_meter/main.c b/examples/qos_meter/main.c
> index b968b00..7c69606 100644
> --- a/examples/qos_meter/main.c
> +++ b/examples/qos_meter/main.c
> @@ -133,14 +133,17 @@ struct rte_meter_trtcm_params
> app_trtcm_params[] = {
>
> FLOW_METER app_flows[APP_FLOWS_MAX];
>
> -static void
> +static int
> app_configure_flow_table(void)
> {
> uint32_t i, j;
> + int ret = 0;
>
> - for (i = 0, j = 0; i < APP_FLOWS_MAX; i ++, j = (j + 1) %
> RTE_DIM(PARAMS)){
> - FUNC_CONFIG(_flows[i], [j]);
> - }
> + for (i = 0, j = 0; i < APP_FLOWS_MAX && ret == 0;
> + i ++, j = (j + 1) % RTE_DIM(PARAMS))
> + ret = FUNC_CONFIG(_flows[i], [j]);
> +
> + return ret;
> }
This is only returns the configuration status for the last flow and leaves
undetected an error for any other flow. Why not check the status for each flow
and return an error on first occurrence?
For (...){ret = FUNC_CONFIG(...); if (ret) return ret;}
>
> static inline void
> @@ -381,7 +384,9 @@ main(int argc, char **argv)
> rte_eth_promiscuous_enable(port_tx);
>
> /* App configuration */
> - app_configure_flow_table();
> + ret = app_configure_flow_table();
> + if (ret < 0)
> + rte_exit(EXIT_FAILURE, "Invalid configure flow table\n");
>
> /* Launch per-lcore init on every lcore */
> rte_eal_mp_remote_launch(main_loop, NULL, CALL_MASTER);
> diff --git a/examples/qos_meter/main.h b/examples/qos_meter/main.h
> index 530bf69..54867dc 100644
> --- a/examples/qos_meter/main.h
> +++ b/examples/qos_meter/main.h
> @@ -51,7 +51,7 @@ enum policer_action
> policer_table[e_RTE_METER_COLORS][e_RTE_METER_COLORS] =
> #if APP_MODE == APP_MODE_FWD
>
> #define FUNC_METER(a,b,c,d) color, flow_id=flow_id, pkt_len=pkt_len,
> time=time
> -#define FUNC_CONFIG(a,b)
> +#define FUNC_CONFIG(a, b) 0
> #define PARAMS app_srtcm_params
> #define FLOW_METER int
>
> --
> 1.9.1
[dpdk-dev] [PATCH v3] examples/qos_meter: fix unchecked return value
Fix issue reported by Coverity.
Coverity ID 30693: Unchecked return value
check_return: Calling rte_meter_srtcm_config without checking return value.
Fixes: e6541fdec8b2 ("meter: initial import")
Signed-off-by: Slawomir Mrozowicz
---
examples/qos_meter/main.c | 15 ++-
examples/qos_meter/main.h | 2 +-
2 files changed, 11 insertions(+), 6 deletions(-)
diff --git a/examples/qos_meter/main.c b/examples/qos_meter/main.c
index b968b00..7c69606 100644
--- a/examples/qos_meter/main.c
+++ b/examples/qos_meter/main.c
@@ -133,14 +133,17 @@ struct rte_meter_trtcm_params app_trtcm_params[] = {
FLOW_METER app_flows[APP_FLOWS_MAX];
-static void
+static int
app_configure_flow_table(void)
{
uint32_t i, j;
+ int ret = 0;
- for (i = 0, j = 0; i < APP_FLOWS_MAX; i ++, j = (j + 1) %
RTE_DIM(PARAMS)){
- FUNC_CONFIG(_flows[i], [j]);
- }
+ for (i = 0, j = 0; i < APP_FLOWS_MAX && ret == 0;
+ i ++, j = (j + 1) % RTE_DIM(PARAMS))
+ ret = FUNC_CONFIG(_flows[i], [j]);
+
+ return ret;
}
static inline void
@@ -381,7 +384,9 @@ main(int argc, char **argv)
rte_eth_promiscuous_enable(port_tx);
/* App configuration */
- app_configure_flow_table();
+ ret = app_configure_flow_table();
+ if (ret < 0)
+ rte_exit(EXIT_FAILURE, "Invalid configure flow table\n");
/* Launch per-lcore init on every lcore */
rte_eal_mp_remote_launch(main_loop, NULL, CALL_MASTER);
diff --git a/examples/qos_meter/main.h b/examples/qos_meter/main.h
index 530bf69..54867dc 100644
--- a/examples/qos_meter/main.h
+++ b/examples/qos_meter/main.h
@@ -51,7 +51,7 @@ enum policer_action
policer_table[e_RTE_METER_COLORS][e_RTE_METER_COLORS] =
#if APP_MODE == APP_MODE_FWD
#define FUNC_METER(a,b,c,d) color, flow_id=flow_id, pkt_len=pkt_len, time=time
-#define FUNC_CONFIG(a,b)
+#define FUNC_CONFIG(a, b) 0
#define PARAMS app_srtcm_params
#define FLOW_METER int
--
1.9.1
