Re: [PATCH] tests/qtest: check the return value

2023-11-21 Thread Thomas Huth 

On 21/11/2023 09.08, Zhu Jun wrote:

These variables "ret" are never referenced in the code, thus
add check logic for the "ret"

Signed-off-by: Zhu Jun 


Thanks!

Reviewed-by: Thomas Huth 

I'll queue it for my next pull request.

RE: [PATCH] tests/qtest: check the return value

2023-11-20 Thread Zhang, Chen 



> -Original Message-
> From: zhujun2 
> Sent: Tuesday, November 21, 2023 2:17 PM
> To: Zhang, Chen 
> Cc: lviv...@redhat.com; pbonz...@redhat.com; qemu-devel@nongnu.org;
> th...@redhat.com; zhuj...@cmss.chinamobile.com
> Subject: [PATCH] tests/qtest: check the return value
> 
> These variables "ret" are never referenced in the code, that add check logic
> for the "ret"
> 

Just tiny comments, please check the len before str, and add the V2 tag in mail 
title next time.
After fix my comments and Thomas comments:
Reviewed-by: Zhang Chen 

Thanks
Chen

> Signed-off-by: zhujun2 
> ---
>  tests/qtest/test-filter-mirror.c | 1 +
>  tests/qtest/test-filter-redirector.c | 2 ++
>  tests/qtest/virtio-net-test.c| 1 +
>  3 files changed, 4 insertions(+)
> 
> diff --git a/tests/qtest/test-filter-mirror.c 
> b/tests/qtest/test-filter-mirror.c
> index adeada3eb8..f3865f7519 100644
> --- a/tests/qtest/test-filter-mirror.c
> +++ b/tests/qtest/test-filter-mirror.c
> @@ -61,6 +61,7 @@ static void test_mirror(void)
>  g_assert_cmpint(len, ==, sizeof(send_buf));
>  recv_buf = g_malloc(len);
>  ret = recv(recv_sock[0], recv_buf, len, 0);
> +g_assert_cmpint(ret, ==, len);
>  g_assert_cmpstr(recv_buf, ==, send_buf);
> 
>  g_free(recv_buf);
> diff --git a/tests/qtest/test-filter-redirector.c b/tests/qtest/test-filter-
> redirector.c
> index e72e3b7873..a77d5fd8ec 100644
> --- a/tests/qtest/test-filter-redirector.c
> +++ b/tests/qtest/test-filter-redirector.c
> @@ -118,6 +118,7 @@ static void test_redirector_tx(void)
>  g_assert_cmpint(len, ==, sizeof(send_buf));
>  recv_buf = g_malloc(len);
>  ret = recv(recv_sock, recv_buf, len, 0);
> +g_assert_cmpint(ret, ==, len);
>  g_assert_cmpstr(recv_buf, ==, send_buf);
> 
>  g_free(recv_buf);
> @@ -185,6 +186,7 @@ static void test_redirector_rx(void)
>  g_assert_cmpint(len, ==, sizeof(send_buf));
>  recv_buf = g_malloc(len);
>  ret = recv(backend_sock[0], recv_buf, len, 0);
> +g_assert_cmpint(ret, ==, len);
>  g_assert_cmpstr(recv_buf, ==, send_buf);
> 
>  close(send_sock);
> diff --git a/tests/qtest/virtio-net-test.c b/tests/qtest/virtio-net-test.c 
> index
> fab5dd8b05..b470d8c6e2 100644
> --- a/tests/qtest/virtio-net-test.c
> +++ b/tests/qtest/virtio-net-test.c
> @@ -92,6 +92,7 @@ static void tx_test(QVirtioDevice *dev,
> 
>  ret = recv(socket, buffer, len, 0);
>  g_assert_cmpstr(buffer, ==, "TEST");
> +g_assert_cmpint(ret, ==, len);

Move it before g_assert_cmpstr().

>  }
> 
>  static void rx_stop_cont_test(QVirtioDevice *dev,
> --
> 2.17.1
> 
>

Re: [PATCH] tests/qtest: check the return value

2023-11-20 Thread Thomas Huth 



Thanks, patch looks better this way, but I'd still like to ask you to fix 
two small issues:


On 21/11/2023 07.16, zhujun2 wrote:

These variables "ret" are never referenced in the code, that


s/, that/, thus/


add check logic for the "ret"

Signed-off-by: zhujun2 


Could you *please* fix your ~/.gitconfig file, like we asked you last time 
already:



https://lore.kernel.org/qemu-devel/27bd1017-fd0a-4e90-ae54-6064b434b...@redhat.com/

 https://lore.kernel.org/qemu-devel/87sf5w198d@pond.sub.org/

 Thanks,
  Thomas



---
  tests/qtest/test-filter-mirror.c | 1 +
  tests/qtest/test-filter-redirector.c | 2 ++
  tests/qtest/virtio-net-test.c| 1 +
  3 files changed, 4 insertions(+)

diff --git a/tests/qtest/test-filter-mirror.c b/tests/qtest/test-filter-mirror.c
index adeada3eb8..f3865f7519 100644
--- a/tests/qtest/test-filter-mirror.c
+++ b/tests/qtest/test-filter-mirror.c
@@ -61,6 +61,7 @@ static void test_mirror(void)
  g_assert_cmpint(len, ==, sizeof(send_buf));
  recv_buf = g_malloc(len);
  ret = recv(recv_sock[0], recv_buf, len, 0);
+g_assert_cmpint(ret, ==, len);
  g_assert_cmpstr(recv_buf, ==, send_buf);
  
  g_free(recv_buf);

diff --git a/tests/qtest/test-filter-redirector.c 
b/tests/qtest/test-filter-redirector.c
index e72e3b7873..a77d5fd8ec 100644
--- a/tests/qtest/test-filter-redirector.c
+++ b/tests/qtest/test-filter-redirector.c
@@ -118,6 +118,7 @@ static void test_redirector_tx(void)
  g_assert_cmpint(len, ==, sizeof(send_buf));
  recv_buf = g_malloc(len);
  ret = recv(recv_sock, recv_buf, len, 0);
+g_assert_cmpint(ret, ==, len);
  g_assert_cmpstr(recv_buf, ==, send_buf);
  
  g_free(recv_buf);

@@ -185,6 +186,7 @@ static void test_redirector_rx(void)
  g_assert_cmpint(len, ==, sizeof(send_buf));
  recv_buf = g_malloc(len);
  ret = recv(backend_sock[0], recv_buf, len, 0);
+g_assert_cmpint(ret, ==, len);
  g_assert_cmpstr(recv_buf, ==, send_buf);
  
  close(send_sock);

diff --git a/tests/qtest/virtio-net-test.c b/tests/qtest/virtio-net-test.c
index fab5dd8b05..b470d8c6e2 100644
--- a/tests/qtest/virtio-net-test.c
+++ b/tests/qtest/virtio-net-test.c
@@ -92,6 +92,7 @@ static void tx_test(QVirtioDevice *dev,
  
  ret = recv(socket, buffer, len, 0);

  g_assert_cmpstr(buffer, ==, "TEST");
+g_assert_cmpint(ret, ==, len);
  }
  
  static void rx_stop_cont_test(QVirtioDevice *dev,