tests: Fix some minor Coverity issues

[PATCH 6/6] tests/unit/test-vmstate: Assert that dup() and mkstemp() succeed

Posted by Peter Maydell 4 years, 1 month ago

Coverity complains that we don't check for failures from dup()
and mkstemp(); add asserts that these syscalls succeeded.

Fixes: Coverity CID 1432516, 1432574
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 tests/unit/test-vmstate.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tests/unit/test-vmstate.c b/tests/unit/test-vmstate.c
index a001879585e..8d46af61511 100644
--- a/tests/unit/test-vmstate.c
+++ b/tests/unit/test-vmstate.c
@@ -44,6 +44,7 @@ static QEMUFile *open_test_file(bool write)
     QIOChannel *ioc;
     QEMUFile *f;
 
+    g_assert(fd >= 0);
     lseek(fd, 0, SEEK_SET);
     if (write) {
         g_assert_cmpint(ftruncate(fd, 0), ==, 0);
@@ -1486,6 +1487,7 @@ int main(int argc, char **argv)
     g_autofree char *temp_file = g_strdup_printf("%s/vmst.test.XXXXXX",
                                                  g_get_tmp_dir());
     temp_fd = mkstemp(temp_file);
+    g_assert(temp_fd >= 0);
 
     module_call_init(MODULE_INIT_QOM);
 
-- 
2.20.1

Re: [PATCH 6/6] tests/unit/test-vmstate: Assert that dup() and mkstemp() succeed

Posted by Philippe Mathieu-Daudé 4 years, 1 month ago

On 5/25/21 3:44 PM, Peter Maydell wrote:
> Coverity complains that we don't check for failures from dup()
> and mkstemp(); add asserts that these syscalls succeeded.
> 
> Fixes: Coverity CID 1432516, 1432574
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  tests/unit/test-vmstate.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/tests/unit/test-vmstate.c b/tests/unit/test-vmstate.c
> index a001879585e..8d46af61511 100644
> --- a/tests/unit/test-vmstate.c
> +++ b/tests/unit/test-vmstate.c
> @@ -44,6 +44,7 @@ static QEMUFile *open_test_file(bool write)
>      QIOChannel *ioc;
>      QEMUFile *f;
>  

Eventually move the assignation before the assertion:

-- >8 --
@@ -40,10 +40,12 @@ static int temp_fd;
 /* Duplicate temp_fd and seek to the beginning of the file */
 static QEMUFile *open_test_file(bool write)
 {
-    int fd = dup(temp_fd);
+    int fd;
     QIOChannel *ioc;
     QEMUFile *f;

+    fd = dup(temp_fd);
+    g_assert(fd >= 0);
     lseek(fd, 0, SEEK_SET);
     if (write) {
         g_assert_cmpint(ftruncate(fd, 0), ==, 0);

---

Regardless:
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> +    g_assert(fd >= 0);
>      lseek(fd, 0, SEEK_SET);
>      if (write) {
>          g_assert_cmpint(ftruncate(fd, 0), ==, 0);
> @@ -1486,6 +1487,7 @@ int main(int argc, char **argv)
>      g_autofree char *temp_file = g_strdup_printf("%s/vmst.test.XXXXXX",
>                                                   g_get_tmp_dir());
>      temp_fd = mkstemp(temp_file);
> +    g_assert(temp_fd >= 0);
>  
>      module_call_init(MODULE_INIT_QOM);
>  
>

Re: [PATCH 6/6] tests/unit/test-vmstate: Assert that dup() and mkstemp() succeed

Posted by Stefan Berger 4 years, 1 month ago

On 5/25/21 9:44 AM, Peter Maydell wrote:
> Coverity complains that we don't check for failures from dup()
> and mkstemp(); add asserts that these syscalls succeeded.
>
> Fixes: Coverity CID 1432516, 1432574
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>


> ---
>   tests/unit/test-vmstate.c | 2 ++
>   1 file changed, 2 insertions(+)
>
> diff --git a/tests/unit/test-vmstate.c b/tests/unit/test-vmstate.c
> index a001879585e..8d46af61511 100644
> --- a/tests/unit/test-vmstate.c
> +++ b/tests/unit/test-vmstate.c
> @@ -44,6 +44,7 @@ static QEMUFile *open_test_file(bool write)
>       QIOChannel *ioc;
>       QEMUFile *f;
>
> +    g_assert(fd >= 0);
>       lseek(fd, 0, SEEK_SET);
>       if (write) {
>           g_assert_cmpint(ftruncate(fd, 0), ==, 0);
> @@ -1486,6 +1487,7 @@ int main(int argc, char **argv)
>       g_autofree char *temp_file = g_strdup_printf("%s/vmst.test.XXXXXX",
>                                                    g_get_tmp_dir());
>       temp_fd = mkstemp(temp_file);
> +    g_assert(temp_fd >= 0);
>
>       module_call_init(MODULE_INIT_QOM);
>

[PATCH 1/6] tests/qtest/bios-tables-test: Check for dup2() failure
[PATCH 2/6] tests/qtest/e1000e-test: Check qemu_recv() succeeded
[PATCH 3/6] tests/qtest/hd-geo-test: Fix checks on mkstemp() return value
[PATCH 4/6] tests/qtest/pflash-cfi02-test: Avoid potential integer overflow
[PATCH 5/6] tests/qtest/tpm-tests: Remove unnecessary NULL checks
[PATCH 6/6] tests/unit/test-vmstate: Assert that dup() and mkstemp() succeed