| Message ID | 1459762145-40697-1-git-send-email-christophe.milard@linaro.org |
|---|---|
| State | New |
| Headers | show |
On Mon, Apr 4, 2016 at 4:29 AM, Christophe Milard < christophe.milard@linaro.org> wrote: > Fixes: https://bugs.linaro.org/show_bug.cgi?id=2148 (CID 159393) > The if statement introduced here is redundant with the previous > CU_ASSERT_FATAL, but avoids the coverity warning. > (coverity probably misses the longjump in CU_ASSERT_FATAL) > > Signed-off-by: Christophe Milard <christophe.milard@linaro.org> > Reviewed-by: Bill Fischofer <bill.fischofer@linaro.org> > --- > > NOTE: to be applied on to of: "linux-generic: test: shmem: close fifo" > > platform/linux-generic/test/shmem/shmem_odp.c | 8 +++++--- > 1 file changed, 5 insertions(+), 3 deletions(-) > > diff --git a/platform/linux-generic/test/shmem/shmem_odp.c > b/platform/linux-generic/test/shmem/shmem_odp.c > index a1f750f..77c5a01 100644 > --- a/platform/linux-generic/test/shmem/shmem_odp.c > +++ b/platform/linux-generic/test/shmem/shmem_odp.c > @@ -47,9 +47,11 @@ void shmem_test_odp_shm_proc(void) > fd = open(fifo_name, O_RDONLY); > CU_ASSERT_FATAL(fd >= 0); > > - CU_ASSERT(read(fd, &test_result, sizeof(char)) == 1); > - close(fd); > - CU_ASSERT_FATAL(test_result == TEST_SUCCESS); > + if (fd >= 0) { /* redundant, but to avoid coverity CID 159393 */ > + CU_ASSERT_FATAL(read(fd, &test_result, sizeof(char)) == 1); > + close(fd); > + CU_ASSERT_FATAL(test_result == TEST_SUCCESS); > + } > > CU_ASSERT(odp_shm_free(shm) == 0); > } > -- > 2.1.4 > >
Ping: time for merge? On 4 April 2016 at 18:23, Bill Fischofer <bill.fischofer@linaro.org> wrote: > > > On Mon, Apr 4, 2016 at 4:29 AM, Christophe Milard < > christophe.milard@linaro.org> wrote: > >> Fixes: https://bugs.linaro.org/show_bug.cgi?id=2148 (CID 159393) >> The if statement introduced here is redundant with the previous >> CU_ASSERT_FATAL, but avoids the coverity warning. >> (coverity probably misses the longjump in CU_ASSERT_FATAL) >> >> Signed-off-by: Christophe Milard <christophe.milard@linaro.org> >> > > Reviewed-by: Bill Fischofer <bill.fischofer@linaro.org> > > >> --- >> >> NOTE: to be applied on to of: "linux-generic: test: shmem: close fifo" >> >> platform/linux-generic/test/shmem/shmem_odp.c | 8 +++++--- >> 1 file changed, 5 insertions(+), 3 deletions(-) >> >> diff --git a/platform/linux-generic/test/shmem/shmem_odp.c >> b/platform/linux-generic/test/shmem/shmem_odp.c >> index a1f750f..77c5a01 100644 >> --- a/platform/linux-generic/test/shmem/shmem_odp.c >> +++ b/platform/linux-generic/test/shmem/shmem_odp.c >> @@ -47,9 +47,11 @@ void shmem_test_odp_shm_proc(void) >> fd = open(fifo_name, O_RDONLY); >> CU_ASSERT_FATAL(fd >= 0); >> >> - CU_ASSERT(read(fd, &test_result, sizeof(char)) == 1); >> - close(fd); >> - CU_ASSERT_FATAL(test_result == TEST_SUCCESS); >> + if (fd >= 0) { /* redundant, but to avoid coverity CID 159393 */ >> + CU_ASSERT_FATAL(read(fd, &test_result, sizeof(char)) == >> 1); >> + close(fd); >> + CU_ASSERT_FATAL(test_result == TEST_SUCCESS); >> + } >> >> CU_ASSERT(odp_shm_free(shm) == 0); >> } >> -- >> 2.1.4 >> >> >
On 04/04/16 12:29, Christophe Milard wrote: > Fixes: https://bugs.linaro.org/show_bug.cgi?id=2148 (CID 159393) > The if statement introduced here is redundant with the previous > CU_ASSERT_FATAL, but avoids the coverity warning. > (coverity probably misses the longjump in CU_ASSERT_FATAL) > > Signed-off-by: Christophe Milard <christophe.milard@linaro.org> > --- > > NOTE: to be applied on to of: "linux-generic: test: shmem: close fifo" > > platform/linux-generic/test/shmem/shmem_odp.c | 8 +++++--- > 1 file changed, 5 insertions(+), 3 deletions(-) > > diff --git a/platform/linux-generic/test/shmem/shmem_odp.c b/platform/linux-generic/test/shmem/shmem_odp.c > index a1f750f..77c5a01 100644 > --- a/platform/linux-generic/test/shmem/shmem_odp.c > +++ b/platform/linux-generic/test/shmem/shmem_odp.c > @@ -47,9 +47,11 @@ void shmem_test_odp_shm_proc(void) > fd = open(fifo_name, O_RDONLY); > CU_ASSERT_FATAL(fd >= 0); > > - CU_ASSERT(read(fd, &test_result, sizeof(char)) == 1); > - close(fd); > - CU_ASSERT_FATAL(test_result == TEST_SUCCESS); > + if (fd >= 0) { /* redundant, but to avoid coverity CID 159393 */ CID number should be removed from test code as well as all that comment. Also you are breaking test logic. I.e. if (fd == 0) then you don't do read() and test test_result. And in that case test will be successful. So I think only think you need is to add ASSERTS for fd and close(fd). Regards, Maxim. > + CU_ASSERT_FATAL(read(fd, &test_result, sizeof(char)) == 1); > + close(fd); > + CU_ASSERT_FATAL(test_result == TEST_SUCCESS); > + } > > CU_ASSERT(odp_shm_free(shm) == 0); > }
We decided to drop this patch and mark it as false positive in Coverity. Maxim. On 04/14/16 15:49, Christophe Milard wrote: > Ping: time for merge? > > On 4 April 2016 at 18:23, Bill Fischofer <bill.fischofer@linaro.org > <mailto:bill.fischofer@linaro.org>> wrote: > > > > On Mon, Apr 4, 2016 at 4:29 AM, Christophe Milard > <christophe.milard@linaro.org > <mailto:christophe.milard@linaro.org>> wrote: > > Fixes: https://bugs.linaro.org/show_bug.cgi?id=2148 (CID 159393) > The if statement introduced here is redundant with the previous > CU_ASSERT_FATAL, but avoids the coverity warning. > (coverity probably misses the longjump in CU_ASSERT_FATAL) > > Signed-off-by: Christophe Milard <christophe.milard@linaro.org > <mailto:christophe.milard@linaro.org>> > > > Reviewed-by: Bill Fischofer <bill.fischofer@linaro.org > <mailto:bill.fischofer@linaro.org>> > > --- > > NOTE: to be applied on to of: "linux-generic: test: shmem: > close fifo" > > platform/linux-generic/test/shmem/shmem_odp.c | 8 +++++--- > 1 file changed, 5 insertions(+), 3 deletions(-) > > diff --git a/platform/linux-generic/test/shmem/shmem_odp.c > b/platform/linux-generic/test/shmem/shmem_odp.c > index a1f750f..77c5a01 100644 > --- a/platform/linux-generic/test/shmem/shmem_odp.c > +++ b/platform/linux-generic/test/shmem/shmem_odp.c > @@ -47,9 +47,11 @@ void shmem_test_odp_shm_proc(void) > fd = open(fifo_name, O_RDONLY); > CU_ASSERT_FATAL(fd >= 0); > > - CU_ASSERT(read(fd, &test_result, sizeof(char)) == 1); > - close(fd); > - CU_ASSERT_FATAL(test_result == TEST_SUCCESS); > + if (fd >= 0) { /* redundant, but to avoid coverity > CID 159393 */ > + CU_ASSERT_FATAL(read(fd, &test_result, > sizeof(char)) == 1); > + close(fd); > + CU_ASSERT_FATAL(test_result == TEST_SUCCESS); > + } > > CU_ASSERT(odp_shm_free(shm) == 0); > } > -- > 2.1.4 > > >
diff --git a/platform/linux-generic/test/shmem/shmem_odp.c b/platform/linux-generic/test/shmem/shmem_odp.c index a1f750f..77c5a01 100644 --- a/platform/linux-generic/test/shmem/shmem_odp.c +++ b/platform/linux-generic/test/shmem/shmem_odp.c @@ -47,9 +47,11 @@ void shmem_test_odp_shm_proc(void) fd = open(fifo_name, O_RDONLY); CU_ASSERT_FATAL(fd >= 0); - CU_ASSERT(read(fd, &test_result, sizeof(char)) == 1); - close(fd); - CU_ASSERT_FATAL(test_result == TEST_SUCCESS); + if (fd >= 0) { /* redundant, but to avoid coverity CID 159393 */ + CU_ASSERT_FATAL(read(fd, &test_result, sizeof(char)) == 1); + close(fd); + CU_ASSERT_FATAL(test_result == TEST_SUCCESS); + } CU_ASSERT(odp_shm_free(shm) == 0); }
Fixes: https://bugs.linaro.org/show_bug.cgi?id=2148 (CID 159393) The if statement introduced here is redundant with the previous CU_ASSERT_FATAL, but avoids the coverity warning. (coverity probably misses the longjump in CU_ASSERT_FATAL) Signed-off-by: Christophe Milard <christophe.milard@linaro.org> --- NOTE: to be applied on to of: "linux-generic: test: shmem: close fifo" platform/linux-generic/test/shmem/shmem_odp.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)