[PATCH 1/5] tests/qtest/ipmi-kcs: Fix assert side-effect

Philippe Mathieu-Daudé posted 5 patches 5 years, 2 months ago
[PATCH 1/5] tests/qtest/ipmi-kcs: Fix assert side-effect
Posted by Philippe Mathieu-Daudé 5 years, 2 months ago
Fix assert side-effect reported by Coverity:

  /qemu/tests/qtest/ipmi-kcs-test.c: 84 in kcs_wait_obf()
  83         while (IPMI_KCS_CMDREG_GET_OBF() == 0) {
  >>>     CID 1432368:  Incorrect expression  (ASSERT_SIDE_EFFECT)
  >>>     Argument "--count" of g_assert() has a side effect.  The containing function might work differently in a non-debug build.
  84             g_assert(--count != 0);

Reported-by: Coverity (CID 1432368)
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 tests/qtest/ipmi-kcs-test.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tests/qtest/ipmi-kcs-test.c b/tests/qtest/ipmi-kcs-test.c
index 693a6aacb52..fc0a918c8d1 100644
--- a/tests/qtest/ipmi-kcs-test.c
+++ b/tests/qtest/ipmi-kcs-test.c
@@ -81,7 +81,8 @@ static void kcs_wait_obf(void)
 {
     unsigned int count = 1000;
     while (IPMI_KCS_CMDREG_GET_OBF() == 0) {
-        g_assert(--count != 0);
+        --count;
+        g_assert(count != 0);
     }
 }
 
-- 
2.26.2


Re: [PATCH 1/5] tests/qtest/ipmi-kcs: Fix assert side-effect
Posted by Greg Kurz 5 years, 2 months ago
On Wed,  2 Sep 2020 10:07:57 +0200
Philippe Mathieu-Daudé <philmd@redhat.com> wrote:

> Fix assert side-effect reported by Coverity:
> 
>   /qemu/tests/qtest/ipmi-kcs-test.c: 84 in kcs_wait_obf()
>   83         while (IPMI_KCS_CMDREG_GET_OBF() == 0) {
>   >>>     CID 1432368:  Incorrect expression  (ASSERT_SIDE_EFFECT)
>   >>>     Argument "--count" of g_assert() has a side effect.  The containing function might work differently in a non-debug build.
>   84             g_assert(--count != 0);
> 
> Reported-by: Coverity (CID 1432368)
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---

Reviewed-by: Greg Kurz <groug@kaod.org>

>  tests/qtest/ipmi-kcs-test.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/qtest/ipmi-kcs-test.c b/tests/qtest/ipmi-kcs-test.c
> index 693a6aacb52..fc0a918c8d1 100644
> --- a/tests/qtest/ipmi-kcs-test.c
> +++ b/tests/qtest/ipmi-kcs-test.c
> @@ -81,7 +81,8 @@ static void kcs_wait_obf(void)
>  {
>      unsigned int count = 1000;
>      while (IPMI_KCS_CMDREG_GET_OBF() == 0) {
> -        g_assert(--count != 0);
> +        --count;
> +        g_assert(count != 0);
>      }
>  }
>  


Re: [PATCH 1/5] tests/qtest/ipmi-kcs: Fix assert side-effect
Posted by Thomas Huth 5 years, 2 months ago
On 02/09/2020 10.07, Philippe Mathieu-Daudé wrote:
> Fix assert side-effect reported by Coverity:
> 
>   /qemu/tests/qtest/ipmi-kcs-test.c: 84 in kcs_wait_obf()
>   83         while (IPMI_KCS_CMDREG_GET_OBF() == 0) {
>   >>>     CID 1432368:  Incorrect expression  (ASSERT_SIDE_EFFECT)
>   >>>     Argument "--count" of g_assert() has a side effect.  The containing function might work differently in a non-debug build.
>   84             g_assert(--count != 0);
> 
> Reported-by: Coverity (CID 1432368)
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  tests/qtest/ipmi-kcs-test.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/qtest/ipmi-kcs-test.c b/tests/qtest/ipmi-kcs-test.c
> index 693a6aacb52..fc0a918c8d1 100644
> --- a/tests/qtest/ipmi-kcs-test.c
> +++ b/tests/qtest/ipmi-kcs-test.c
> @@ -81,7 +81,8 @@ static void kcs_wait_obf(void)
>  {
>      unsigned int count = 1000;
>      while (IPMI_KCS_CMDREG_GET_OBF() == 0) {
> -        g_assert(--count != 0);
> +        --count;
> +        g_assert(count != 0);
>      }
>  }
>  
> 

You could also use g_assert_true() instead. Anyway:

Reviewed-by: Thomas Huth <thuth@redhat.com>