diff mbox series

[ibsim,11/23] sim_cmd.c: fix CONSTANT_EXPRESSION_RESULT for dump_route

Message ID 20190102131318.5765-11-honli@redhat.com (mailing list archive)
State Not Applicable
Headers show
Series [ibsim,01/23] move sim_cmd_file into ibsim/sim_cmd.c | expand

Commit Message

Honggang LI Jan. 2, 2019, 1:13 p.m. UTC
node->sw->fdb[to] is uint8_t. So, (node->sw->fdb[to] < 0) is always
false.

Issue was detected by Coverity.

Signed-off-by: Honggang Li <honli@redhat.com>
---
 ibsim/sim_cmd.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Hal Rosenstock Jan. 4, 2019, 12:24 p.m. UTC | #1
On 1/2/2019 8:13 AM, Honggang Li wrote:
> node->sw->fdb[to] is uint8_t. So, (node->sw->fdb[to] < 0) is always
> false.
> 
> Issue was detected by Coverity.
> 
> Signed-off-by: Honggang Li <honli@redhat.com>
> ---
>  ibsim/sim_cmd.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/ibsim/sim_cmd.c b/ibsim/sim_cmd.c
> index e2fe6ba6dddb..1d36c600814f 100644
> --- a/ibsim/sim_cmd.c
> +++ b/ibsim/sim_cmd.c
> @@ -751,8 +751,8 @@ static int dump_route(FILE * f, char *line)
>  			break;	// found
>  		outport = portnum;
>  		if (node->type == SWITCH_NODE) {
> -			if ((outport = node->sw->fdb[to]) < 0
> -			    || to > node->sw->linearFDBtop)
> +			outport = node->sw->fdb[to];
> +			if (to > node->sw->linearFDBtop)

This is not the proper fix. I think that the check should be == 0xff
rather than < 0 or eliminated as in this patch since fdb is initialized
to 0xff (no port for LID).

-- Hal

>  				goto badtbl;
>  			port = ports + node->portsbase + outport;
>  			if (outport == 0) {
>
Honggang LI Jan. 4, 2019, 2:34 p.m. UTC | #2
On Fri, Jan 04, 2019 at 07:24:08AM -0500, Hal Rosenstock wrote:
> On 1/2/2019 8:13 AM, Honggang Li wrote:
> > node->sw->fdb[to] is uint8_t. So, (node->sw->fdb[to] < 0) is always
> > false.
> > 
> > Issue was detected by Coverity.
> > 
> > Signed-off-by: Honggang Li <honli@redhat.com>
> > ---
> >  ibsim/sim_cmd.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/ibsim/sim_cmd.c b/ibsim/sim_cmd.c
> > index e2fe6ba6dddb..1d36c600814f 100644
> > --- a/ibsim/sim_cmd.c
> > +++ b/ibsim/sim_cmd.c
> > @@ -751,8 +751,8 @@ static int dump_route(FILE * f, char *line)
> >  			break;	// found
> >  		outport = portnum;
> >  		if (node->type == SWITCH_NODE) {
> > -			if ((outport = node->sw->fdb[to]) < 0
> > -			    || to > node->sw->linearFDBtop)
> > +			outport = node->sw->fdb[to];
> > +			if (to > node->sw->linearFDBtop)
> 
> This is not the proper fix. I think that the check should be == 0xff
> rather than < 0 or eliminated as in this patch since fdb is initialized
> to 0xff (no port for LID).

Yes, 0xff indicates that an invalid port number was used. It should be:

outport = node->sw->fdb[to]) == 0xff

as you said.

Please fix it.
diff mbox series

Patch

diff --git a/ibsim/sim_cmd.c b/ibsim/sim_cmd.c
index e2fe6ba6dddb..1d36c600814f 100644
--- a/ibsim/sim_cmd.c
+++ b/ibsim/sim_cmd.c
@@ -751,8 +751,8 @@  static int dump_route(FILE * f, char *line)
 			break;	// found
 		outport = portnum;
 		if (node->type == SWITCH_NODE) {
-			if ((outport = node->sw->fdb[to]) < 0
-			    || to > node->sw->linearFDBtop)
+			outport = node->sw->fdb[to];
+			if (to > node->sw->linearFDBtop)
 				goto badtbl;
 			port = ports + node->portsbase + outport;
 			if (outport == 0) {