Date:   Thu, 9 Mar 2023 09:49:20 +0100
From:   Stefano Garzarella <sgarzare@redhat.com>
To:     Rong Tao <rtoax@foxmail.com>
Cc:     Rong Tao <rongtao@cestc.cn>, "Michael S. Tsirkin" <mst@redhat.com>,
        Jason Wang <jasowang@redhat.com>,
        "open list:VIRTIO CORE AND NET DRIVERS" 
        <virtualization@lists.linux-foundation.org>,
        open list <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v2] tools/virtio: virtio_test -h,--help should return
 directly
Message-ID: <20230309084920.oj66qnighnnyet3f@sgarzare-redhat>
References: <tencent_822CEBEB925205EA1573541CD1C2604F4805@qq.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii; format=flowed
Content-Disposition: inline
In-Reply-To: <tencent_822CEBEB925205EA1573541CD1C2604F4805@qq.com>
Precedence: bulk

On Thu, Mar 09, 2023 at 04:42:50PM +0800, Rong Tao wrote:
>From: Rong Tao <rongtao@cestc.cn>
>
>When we get help information, we should return directly, and we should not
>execute test cases. Move the exit() directly into the help() function and
>remove it from case '?'.
>
>Signed-off-by: Rong Tao <rongtao@cestc.cn>
>---
>v2: help(): exit with a value different from 0 (e.g. 2).
>v1: https://lore.kernel.org/lkml/tencent_4B5122C4158323A1D1ACA04B3295F1579207@qq.com/
>---
> tools/virtio/virtio_test.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
>diff --git a/tools/virtio/virtio_test.c b/tools/virtio/virtio_test.c
>index 120062f94590..33e17307441f 100644
>--- a/tools/virtio/virtio_test.c
>+++ b/tools/virtio/virtio_test.c
>@@ -327,7 +327,7 @@ const struct option longopts[] = {
> 	}
> };
>
>-static void help(void)
>+static void help(int status)
> {
> 	fprintf(stderr, "Usage: virtio_test [--help]"
> 		" [--no-indirect]"
>@@ -337,6 +337,8 @@ static void help(void)
> 		" [--batch=random/N]"
> 		" [--reset=N]"
> 		"\n");
>+
>+	exit(status);

Sorry, I meant you can put exit(2) here, and remove the `int status` 
parameter of help().

> }
>
> int main(int argc, char **argv)
>@@ -354,14 +356,12 @@ int main(int argc, char **argv)
> 		case -1:
> 			goto done;
> 		case '?':
>-			help();
>-			exit(2);
>+			help(2);

Here you can use help()

> 		case 'e':
> 			features &= ~(1ULL << VIRTIO_RING_F_EVENT_IDX);
> 			break;
> 		case 'h':
>-			help();
>-			goto done;
>+			help(0);

And also here you can use help()

I'm sure you're learning the process, so as a suggestion for the future,
if some thing is not clear, better to ask before sending another
version.

Thanks,
Stefano

> 		case 'i':
> 			features &= ~(1ULL << 
> 			VIRTIO_RING_F_INDIRECT_DESC);
> 			break;
>-- 
>2.39.1
>