| Message ID | 1424664603-29684-1-git-send-email-bala.manoharan@linaro.org |
|---|---|
| State | Accepted |
| Commit | 31533eb9cac3a141865a5f9c897f6e4320c96665 |
| Headers | show |
On Sun, Feb 22, 2015 at 10:10 PM, <bala.manoharan@linaro.org> wrote: > From: Balasubramanian Manoharan <bala.manoharan@linaro.org> > > Fixes uninitialized scalar variable qos_tbl in > configure_cos_with_l2_priority() > function reported by Coverity. > > https://bugs.linaro.org/show_bug.cgi?id=1143 > > Signed-off-by: Balasubramanian Manoharan <bala.manoharan@linaro.org> > Reviewed-by: Bill Fischofer <bill.fischofer@linaro.org> > --- > test/validation/classification/odp_classification_tests.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/test/validation/classification/odp_classification_tests.c > b/test/validation/classification/odp_classification_tests.c > index 564455c..4807584 100644 > --- a/test/validation/classification/odp_classification_tests.c > +++ b/test/validation/classification/odp_classification_tests.c > @@ -555,6 +555,10 @@ void configure_cos_with_l2_priority(void) > int i; > odp_queue_param_t qparam; > > + /** Initialize scalar variable qos_tbl **/ > + for (i = 0; i < CLS_L2_QOS_MAX; i++) > + qos_tbl[i] = 0; > + > qparam.sched.sync = ODP_SCHED_SYNC_NONE; > qparam.sched.group = ODP_SCHED_GROUP_ALL; > for (i = 0; i < num_qos; i++) { > -- > 2.0.1.472.g6f92e5f > > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/lng-odp >
On 02/23/2015 07:10 AM, bala.manoharan@linaro.org wrote: > From: Balasubramanian Manoharan <bala.manoharan@linaro.org> > > Fixes uninitialized scalar variable qos_tbl in configure_cos_with_l2_priority() > function reported by Coverity. > > https://bugs.linaro.org/show_bug.cgi?id=1143 > > Signed-off-by: Balasubramanian Manoharan <bala.manoharan@linaro.org> > --- > test/validation/classification/odp_classification_tests.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/test/validation/classification/odp_classification_tests.c b/test/validation/classification/odp_classification_tests.c > index 564455c..4807584 100644 > --- a/test/validation/classification/odp_classification_tests.c > +++ b/test/validation/classification/odp_classification_tests.c > @@ -555,6 +555,10 @@ void configure_cos_with_l2_priority(void) > int i; > odp_queue_param_t qparam; > > + /** Initialize scalar variable qos_tbl **/ > + for (i = 0; i < CLS_L2_QOS_MAX; i++) > + qos_tbl[i] = 0; > + > qparam.sched.sync = ODP_SCHED_SYNC_NONE; > qparam.sched.group = ODP_SCHED_GROUP_ALL; > for (i = 0; i < num_qos; i++) { Bala, one small note. In first case "for .. CLS_L2_QOS_MAX", in second case "for num_qos", which has the same value. Just to be consistent can you remove num_qos variable from that function? It's not needed. Maxim.
On 02/24/2015 11:14 AM, Maxim Uvarov wrote: > On 02/23/2015 07:10 AM, bala.manoharan@linaro.org wrote: >> From: Balasubramanian Manoharan <bala.manoharan@linaro.org> >> >> Fixes uninitialized scalar variable qos_tbl in >> configure_cos_with_l2_priority() >> function reported by Coverity. >> >> https://bugs.linaro.org/show_bug.cgi?id=1143 >> >> Signed-off-by: Balasubramanian Manoharan <bala.manoharan@linaro.org> >> --- >> test/validation/classification/odp_classification_tests.c | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git >> a/test/validation/classification/odp_classification_tests.c >> b/test/validation/classification/odp_classification_tests.c >> index 564455c..4807584 100644 >> --- a/test/validation/classification/odp_classification_tests.c >> +++ b/test/validation/classification/odp_classification_tests.c >> @@ -555,6 +555,10 @@ void configure_cos_with_l2_priority(void) >> int i; >> odp_queue_param_t qparam; >> + /** Initialize scalar variable qos_tbl **/ >> + for (i = 0; i < CLS_L2_QOS_MAX; i++) >> + qos_tbl[i] = 0; >> + >> qparam.sched.sync = ODP_SCHED_SYNC_NONE; >> qparam.sched.group = ODP_SCHED_GROUP_ALL; >> for (i = 0; i < num_qos; i++) { > Bala, one small note. In first case "for .. CLS_L2_QOS_MAX", in > second case "for num_qos", which has the same value. > Just to be consistent can you remove num_qos variable from that > function? It's not needed. > > Maxim. > > And might be that code will be better:: if (cos_tbl[i] == ODP_COS_INVALID) { qos_tbl[i] = 0; break; } Maxim.
On Tuesday 24 February 2015 01:44 PM, Maxim Uvarov wrote: > On 02/23/2015 07:10 AM, bala.manoharan@linaro.org wrote: >> From: Balasubramanian Manoharan <bala.manoharan@linaro.org> >> >> Fixes uninitialized scalar variable qos_tbl in >> configure_cos_with_l2_priority() >> function reported by Coverity. >> >> https://bugs.linaro.org/show_bug.cgi?id=1143 >> >> Signed-off-by: Balasubramanian Manoharan <bala.manoharan@linaro.org> >> --- >> test/validation/classification/odp_classification_tests.c | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git >> a/test/validation/classification/odp_classification_tests.c >> b/test/validation/classification/odp_classification_tests.c >> index 564455c..4807584 100644 >> --- a/test/validation/classification/odp_classification_tests.c >> +++ b/test/validation/classification/odp_classification_tests.c >> @@ -555,6 +555,10 @@ void configure_cos_with_l2_priority(void) >> int i; >> odp_queue_param_t qparam; >> + /** Initialize scalar variable qos_tbl **/ >> + for (i = 0; i < CLS_L2_QOS_MAX; i++) >> + qos_tbl[i] = 0; >> + >> qparam.sched.sync = ODP_SCHED_SYNC_NONE; >> qparam.sched.group = ODP_SCHED_GROUP_ALL; >> for (i = 0; i < num_qos; i++) { > Bala, one small note. In first case "for .. CLS_L2_QOS_MAX", in > second case "for num_qos", which has the same value. > Just to be consistent can you remove num_qos variable from that > function? It's not needed. > > Maxim. Makes sense but since this is just sunnyday validation we have executed the API with just one value for "num_qos" but as part of enhancement to validation suite this API needs to be called with different values of "num_qos". So I think it is better to keep num_qos variable. Bala > > > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/lng-odp
On Tuesday 24 February 2015 01:46 PM, Maxim Uvarov wrote: > On 02/24/2015 11:14 AM, Maxim Uvarov wrote: >> On 02/23/2015 07:10 AM, bala.manoharan@linaro.org wrote: >>> From: Balasubramanian Manoharan <bala.manoharan@linaro.org> >>> >>> Fixes uninitialized scalar variable qos_tbl in >>> configure_cos_with_l2_priority() >>> function reported by Coverity. >>> >>> https://bugs.linaro.org/show_bug.cgi?id=1143 >>> >>> Signed-off-by: Balasubramanian Manoharan <bala.manoharan@linaro.org> >>> --- >>> test/validation/classification/odp_classification_tests.c | 4 ++++ >>> 1 file changed, 4 insertions(+) >>> >>> diff --git >>> a/test/validation/classification/odp_classification_tests.c >>> b/test/validation/classification/odp_classification_tests.c >>> index 564455c..4807584 100644 >>> --- a/test/validation/classification/odp_classification_tests.c >>> +++ b/test/validation/classification/odp_classification_tests.c >>> @@ -555,6 +555,10 @@ void configure_cos_with_l2_priority(void) >>> int i; >>> odp_queue_param_t qparam; >>> + /** Initialize scalar variable qos_tbl **/ >>> + for (i = 0; i < CLS_L2_QOS_MAX; i++) >>> + qos_tbl[i] = 0; >>> + >>> qparam.sched.sync = ODP_SCHED_SYNC_NONE; >>> qparam.sched.group = ODP_SCHED_GROUP_ALL; >>> for (i = 0; i < num_qos; i++) { >> Bala, one small note. In first case "for .. CLS_L2_QOS_MAX", in >> second case "for num_qos", which has the same value. >> Just to be consistent can you remove num_qos variable from that >> function? It's not needed. >> >> Maxim. >> >> > > And might be that code will be better:: > if (cos_tbl[i] == ODP_COS_INVALID) { > qos_tbl[i] = 0; > break; > } qos_tbl gets initialized to zero in the beginning so this might not be needed. Regards, Bala > > Maxim. > > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/lng-odp
Bala, can you please send v2 with CLS_L2_QOS_MAX and without num_qos? Or use second option which is faster. Maxim. On 02/24/2015 11:25 AM, Bala wrote: > > On Tuesday 24 February 2015 01:46 PM, Maxim Uvarov wrote: >> On 02/24/2015 11:14 AM, Maxim Uvarov wrote: >>> On 02/23/2015 07:10 AM, bala.manoharan@linaro.org wrote: >>>> From: Balasubramanian Manoharan <bala.manoharan@linaro.org> >>>> >>>> Fixes uninitialized scalar variable qos_tbl in >>>> configure_cos_with_l2_priority() >>>> function reported by Coverity. >>>> >>>> https://bugs.linaro.org/show_bug.cgi?id=1143 >>>> >>>> Signed-off-by: Balasubramanian Manoharan <bala.manoharan@linaro.org> >>>> --- >>>> test/validation/classification/odp_classification_tests.c | 4 ++++ >>>> 1 file changed, 4 insertions(+) >>>> >>>> diff --git >>>> a/test/validation/classification/odp_classification_tests.c >>>> b/test/validation/classification/odp_classification_tests.c >>>> index 564455c..4807584 100644 >>>> --- a/test/validation/classification/odp_classification_tests.c >>>> +++ b/test/validation/classification/odp_classification_tests.c >>>> @@ -555,6 +555,10 @@ void configure_cos_with_l2_priority(void) >>>> int i; >>>> odp_queue_param_t qparam; >>>> + /** Initialize scalar variable qos_tbl **/ >>>> + for (i = 0; i < CLS_L2_QOS_MAX; i++) >>>> + qos_tbl[i] = 0; >>>> + >>>> qparam.sched.sync = ODP_SCHED_SYNC_NONE; >>>> qparam.sched.group = ODP_SCHED_GROUP_ALL; >>>> for (i = 0; i < num_qos; i++) { >>> Bala, one small note. In first case "for .. CLS_L2_QOS_MAX", in >>> second case "for num_qos", which has the same value. >>> Just to be consistent can you remove num_qos variable from that >>> function? It's not needed. >>> >>> Maxim. >>> >>> >> >> And might be that code will be better:: >> if (cos_tbl[i] == ODP_COS_INVALID) { >> qos_tbl[i] = 0; >> break; >> } > qos_tbl gets initialized to zero in the beginning so this might not be > needed. > > Regards, > Bala >> >> Maxim. >> >> >> _______________________________________________ >> lng-odp mailing list >> lng-odp@lists.linaro.org >> http://lists.linaro.org/mailman/listinfo/lng-odp >
On Tuesday 24 February 2015 01:51 PM, Bala wrote: > > On Tuesday 24 February 2015 01:44 PM, Maxim Uvarov wrote: >> On 02/23/2015 07:10 AM, bala.manoharan@linaro.org wrote: >>> From: Balasubramanian Manoharan <bala.manoharan@linaro.org> >>> >>> Fixes uninitialized scalar variable qos_tbl in >>> configure_cos_with_l2_priority() >>> function reported by Coverity. >>> >>> https://bugs.linaro.org/show_bug.cgi?id=1143 >>> >>> Signed-off-by: Balasubramanian Manoharan <bala.manoharan@linaro.org> >>> --- >>> test/validation/classification/odp_classification_tests.c | 4 ++++ >>> 1 file changed, 4 insertions(+) >>> >>> diff --git >>> a/test/validation/classification/odp_classification_tests.c >>> b/test/validation/classification/odp_classification_tests.c >>> index 564455c..4807584 100644 >>> --- a/test/validation/classification/odp_classification_tests.c >>> +++ b/test/validation/classification/odp_classification_tests.c >>> @@ -555,6 +555,10 @@ void configure_cos_with_l2_priority(void) >>> int i; >>> odp_queue_param_t qparam; >>> + /** Initialize scalar variable qos_tbl **/ >>> + for (i = 0; i < CLS_L2_QOS_MAX; i++) >>> + qos_tbl[i] = 0; >>> + >>> qparam.sched.sync = ODP_SCHED_SYNC_NONE; >>> qparam.sched.group = ODP_SCHED_GROUP_ALL; >>> for (i = 0; i < num_qos; i++) { >> Bala, one small note. In first case "for .. CLS_L2_QOS_MAX", in >> second case "for num_qos", which has the same value. >> Just to be consistent can you remove num_qos variable from that >> function? It's not needed. >> >> Maxim. > Makes sense but since this is just sunnyday validation we have > executed the API with just one value for "num_qos" but as part of > enhancement to validation suite this API needs to be called with > different values of "num_qos". So I think it is better to keep num_qos > variable. > > Bala Maxim, I still have the above described concern for this comment. Regards, Bala >> >> >> >> _______________________________________________ >> lng-odp mailing list >> lng-odp@lists.linaro.org >> http://lists.linaro.org/mailman/listinfo/lng-odp >
On 02/24/2015 11:25 AM, Bala wrote: > Bala, one small note. In first case "for .. CLS_L2_QOS_MAX", in > second case "for num_qos", which has the same value. > Just to be consistent can you remove num_qos variable from that > function? It's not needed. On, let it be var for future improvement. But we should not forget about that. Merged, Maxim.
diff --git a/test/validation/classification/odp_classification_tests.c b/test/validation/classification/odp_classification_tests.c index 564455c..4807584 100644 --- a/test/validation/classification/odp_classification_tests.c +++ b/test/validation/classification/odp_classification_tests.c @@ -555,6 +555,10 @@ void configure_cos_with_l2_priority(void) int i; odp_queue_param_t qparam; + /** Initialize scalar variable qos_tbl **/ + for (i = 0; i < CLS_L2_QOS_MAX; i++) + qos_tbl[i] = 0; + qparam.sched.sync = ODP_SCHED_SYNC_NONE; qparam.sched.group = ODP_SCHED_GROUP_ALL; for (i = 0; i < num_qos; i++) {