| Message ID | 1459027277-6414-1-git-send-email-bill.fischofer@linaro.org |
|---|---|
| State | New |
| Headers | show |
On 03/27/16 00:21, Bill Fischofer wrote: > Resolve bug https://bugs.linaro.org/show_bug.cgi?id=2138 by adding an > explicit cast to odp_pktio_capability(). This routine cannot return a > bad RC since the error condition is already guarded earlier in the code, > however coverity doesn't see this. > > Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org> > --- > platform/linux-generic/odp_packet_io.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/platform/linux-generic/odp_packet_io.c b/platform/linux-generic/odp_packet_io.c > index 9192be2..dfeeeaf 100644 > --- a/platform/linux-generic/odp_packet_io.c > +++ b/platform/linux-generic/odp_packet_io.c > @@ -1059,7 +1059,7 @@ int odp_pktin_queue_config(odp_pktio_t pktio, > return -1; > } > > - odp_pktio_capability(pktio, &capa); > + (void)odp_pktio_capability(pktio, &capa); > > if (num_queues > capa.max_input_queues) { if call fails then what value will be in capa.max_input_queues ? I think you can not ignore return code here. > ODP_DBG("pktio %s: too many input queues\n", entry->s.name); > @@ -1172,7 +1172,7 @@ int odp_pktout_queue_config(odp_pktio_t pktio, > return -1; > } > > - odp_pktio_capability(pktio, &capa); > + (void)odp_pktio_capability(pktio, &capa); > > if (num_queues > capa.max_output_queues) { > ODP_DBG("pktio %s: too many output queues\n", entry->s.name); Same thing here. Maxim.
On Fri, Apr 1, 2016 at 11:02 AM, Maxim Uvarov <maxim.uvarov@linaro.org> wrote: > On 03/27/16 00:21, Bill Fischofer wrote: > >> Resolve bug https://bugs.linaro.org/show_bug.cgi?id=2138 by adding an >> explicit cast to odp_pktio_capability(). This routine cannot return a >> bad RC since the error condition is already guarded earlier in the code, >> however coverity doesn't see this. >> >> Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org> >> --- >> platform/linux-generic/odp_packet_io.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/platform/linux-generic/odp_packet_io.c >> b/platform/linux-generic/odp_packet_io.c >> index 9192be2..dfeeeaf 100644 >> --- a/platform/linux-generic/odp_packet_io.c >> +++ b/platform/linux-generic/odp_packet_io.c >> @@ -1059,7 +1059,7 @@ int odp_pktin_queue_config(odp_pktio_t pktio, >> return -1; >> } >> - odp_pktio_capability(pktio, &capa); >> + (void)odp_pktio_capability(pktio, &capa); >> if (num_queues > capa.max_input_queues) { >> > > if call fails then what value will be in capa.max_input_queues ? > I think you can not ignore return code here. > > The call cannot fail in this case because the code is guarded by , but coverity doesn't see this. The cast simply let's coverity know that we're ignoring the return code intentionally. The guard is earlier in the code: entry = get_pktio_entry(pktio); if (entry == NULL) { ODP_DBG("pktio entry %d does not exist\n", pktio); return -1; } The only way odp_pktio_capability() fails is if the PktIO doesn't exist. > > ODP_DBG("pktio %s: too many input queues\n", entry->s.name >> ); >> @@ -1172,7 +1172,7 @@ int odp_pktout_queue_config(odp_pktio_t pktio, >> return -1; >> } >> - odp_pktio_capability(pktio, &capa); >> + (void)odp_pktio_capability(pktio, &capa); >> if (num_queues > capa.max_output_queues) { >> ODP_DBG("pktio %s: too many output queues\n", entry-> >> s.name); >> > Same thing here. > > Maxim. > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > https://lists.linaro.org/mailman/listinfo/lng-odp >
On 04/03/16 15:38, Bill Fischofer wrote: > > > On Fri, Apr 1, 2016 at 11:02 AM, Maxim Uvarov <maxim.uvarov@linaro.org > <mailto:maxim.uvarov@linaro.org>> wrote: > > On 03/27/16 00:21, Bill Fischofer wrote: > > Resolve bug https://bugs.linaro.org/show_bug.cgi?id=2138 by > adding an > explicit cast to odp_pktio_capability(). This routine cannot > return a > bad RC since the error condition is already guarded earlier in > the code, > however coverity doesn't see this. > > Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org > <mailto:bill.fischofer@linaro.org>> > --- > platform/linux-generic/odp_packet_io.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/platform/linux-generic/odp_packet_io.c > b/platform/linux-generic/odp_packet_io.c > index 9192be2..dfeeeaf 100644 > --- a/platform/linux-generic/odp_packet_io.c > +++ b/platform/linux-generic/odp_packet_io.c > @@ -1059,7 +1059,7 @@ int odp_pktin_queue_config(odp_pktio_t > pktio, > return -1; > } > - odp_pktio_capability(pktio, &capa); > + (void)odp_pktio_capability(pktio, &capa); > if (num_queues > capa.max_input_queues) { > > > if call fails then what value will be in capa.max_input_queues ? > I think you can not ignore return code here. > > > The call cannot fail in this case because the code is guarded by , but > coverity doesn't see this. The cast simply let's coverity know that > we're ignoring the return code intentionally. > > The guard is earlier in the code: > > entry = get_pktio_entry(pktio); > if (entry == NULL) { > ODP_DBG("pktio entry %d does not exist\n", pktio); > return -1; > } > > The only way odp_pktio_capability() fails is if the PktIO doesn't exist. if you refer to current implementation you are right. But in somebody will inherit that call from linux-generic and use his own odp_pktio_caps() call that here might be a problem. I think Coverity is right here said to not relay on internal checks for non void functions. Maxim. > > > ODP_DBG("pktio %s: too many input queues\n", > entry->s.name <http://s.name>); > @@ -1172,7 +1172,7 @@ int odp_pktout_queue_config(odp_pktio_t > pktio, > return -1; > } > - odp_pktio_capability(pktio, &capa); > + (void)odp_pktio_capability(pktio, &capa); > if (num_queues > capa.max_output_queues) { > ODP_DBG("pktio %s: too many output queues\n", > entry->s.name <http://s.name>); > > Same thing here. > > Maxim. > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org <mailto:lng-odp@lists.linaro.org> > https://lists.linaro.org/mailman/listinfo/lng-odp > >
Added to today's ARCH call to discuss this. On Mon, Apr 4, 2016 at 1:13 AM, Maxim Uvarov <maxim.uvarov@linaro.org> wrote: > On 04/03/16 15:38, Bill Fischofer wrote: > >> >> >> On Fri, Apr 1, 2016 at 11:02 AM, Maxim Uvarov <maxim.uvarov@linaro.org >> <mailto:maxim.uvarov@linaro.org>> wrote: >> >> On 03/27/16 00:21, Bill Fischofer wrote: >> >> Resolve bug https://bugs.linaro.org/show_bug.cgi?id=2138 by >> adding an >> explicit cast to odp_pktio_capability(). This routine cannot >> return a >> bad RC since the error condition is already guarded earlier in >> the code, >> however coverity doesn't see this. >> >> Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org >> <mailto:bill.fischofer@linaro.org>> >> --- >> platform/linux-generic/odp_packet_io.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/platform/linux-generic/odp_packet_io.c >> b/platform/linux-generic/odp_packet_io.c >> index 9192be2..dfeeeaf 100644 >> --- a/platform/linux-generic/odp_packet_io.c >> +++ b/platform/linux-generic/odp_packet_io.c >> @@ -1059,7 +1059,7 @@ int odp_pktin_queue_config(odp_pktio_t >> pktio, >> return -1; >> } >> - odp_pktio_capability(pktio, &capa); >> + (void)odp_pktio_capability(pktio, &capa); >> if (num_queues > capa.max_input_queues) { >> >> >> if call fails then what value will be in capa.max_input_queues ? >> I think you can not ignore return code here. >> >> >> The call cannot fail in this case because the code is guarded by , but >> coverity doesn't see this. The cast simply let's coverity know that we're >> ignoring the return code intentionally. >> >> The guard is earlier in the code: >> >> entry = get_pktio_entry(pktio); >> if (entry == NULL) { >> ODP_DBG("pktio entry %d does not exist\n", pktio); >> return -1; >> } >> >> The only way odp_pktio_capability() fails is if the PktIO doesn't exist. >> > > if you refer to current implementation you are right. But in somebody will > inherit that call from linux-generic and use his own odp_pktio_caps() call > that here might be a problem. I think Coverity is right here said to not > relay on internal checks for non void > functions. > > Maxim. > >> >> >> ODP_DBG("pktio %s: too many input queues\n", >> entry->s.name <http://s.name>); >> @@ -1172,7 +1172,7 @@ int odp_pktout_queue_config(odp_pktio_t >> pktio, >> return -1; >> } >> - odp_pktio_capability(pktio, &capa); >> + (void)odp_pktio_capability(pktio, &capa); >> if (num_queues > capa.max_output_queues) { >> ODP_DBG("pktio %s: too many output queues\n", >> entry->s.name <http://s.name>); >> >> Same thing here. >> >> Maxim. >> _______________________________________________ >> lng-odp mailing list >> lng-odp@lists.linaro.org <mailto:lng-odp@lists.linaro.org> >> https://lists.linaro.org/mailman/listinfo/lng-odp >> >> >> >
diff --git a/platform/linux-generic/odp_packet_io.c b/platform/linux-generic/odp_packet_io.c index 9192be2..dfeeeaf 100644 --- a/platform/linux-generic/odp_packet_io.c +++ b/platform/linux-generic/odp_packet_io.c @@ -1059,7 +1059,7 @@ int odp_pktin_queue_config(odp_pktio_t pktio, return -1; } - odp_pktio_capability(pktio, &capa); + (void)odp_pktio_capability(pktio, &capa); if (num_queues > capa.max_input_queues) { ODP_DBG("pktio %s: too many input queues\n", entry->s.name); @@ -1172,7 +1172,7 @@ int odp_pktout_queue_config(odp_pktio_t pktio, return -1; } - odp_pktio_capability(pktio, &capa); + (void)odp_pktio_capability(pktio, &capa); if (num_queues > capa.max_output_queues) { ODP_DBG("pktio %s: too many output queues\n", entry->s.name);
Resolve bug https://bugs.linaro.org/show_bug.cgi?id=2138 by adding an explicit cast to odp_pktio_capability(). This routine cannot return a bad RC since the error condition is already guarded earlier in the code, however coverity doesn't see this. Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org> --- platform/linux-generic/odp_packet_io.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)