From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:54790)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <den@openvz.org>) id 1ZUKhz-0000Xy-Gc
	for qemu-devel@nongnu.org; Tue, 25 Aug 2015 16:20:40 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <den@openvz.org>) id 1ZUKhv-0007k1-Hl
	for qemu-devel@nongnu.org; Tue, 25 Aug 2015 16:20:39 -0400
Received: from mx2.parallels.com ([199.115.105.18]:39542)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <den@openvz.org>) id 1ZUKhv-0007i9-CB
	for qemu-devel@nongnu.org; Tue, 25 Aug 2015 16:20:35 -0400
Message-ID: <55DCCE01.2060906@openvz.org>
Date: Tue, 25 Aug 2015 23:20:17 +0300
From: "Denis V. Lunev" <den@openvz.org>
MIME-Version: 1.0
References: <1435751267-26378-1-git-send-email-marcandre.lureau@gmail.com>
	<1435751267-26378-3-git-send-email-marcandre.lureau@gmail.com>
In-Reply-To: <1435751267-26378-3-git-send-email-marcandre.lureau@gmail.com>
Content-Type: text/plain; charset="utf-8"; format=flowed
Content-Transfer-Encoding: 8bit
Subject: Re: [Qemu-devel] [PATCH 02/12] qga: use exit() when parsing options
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: =?UTF-8?B?TWFyYy1BbmRyw6kgTHVyZWF1?= <marcandre.lureau@gmail.com>, qemu-devel@nongnu.org
Cc: mdroth@linux.vnet.ibm.com

On 07/01/2015 02:47 PM, Marc-André Lureau wrote:
> The option parsing is going to be moved to a seperate function,
> use exit() consistantly.
s/seperate/separate/
s/consistantly/consistently/

> Signed-off-by: Marc-André Lureau <marcandre.lureau@gmail.com>
> ---
>   qga/main.c | 24 ++++++++++++------------
>   1 file changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/qga/main.c b/qga/main.c
> index 23cde01..af93992 100644
> --- a/qga/main.c
> +++ b/qga/main.c
> @@ -992,14 +992,14 @@ int main(int argc, char **argv)
>               break;
>           case 'V':
>               printf("QEMU Guest Agent %s\n", QEMU_VERSION);
> -            return 0;
> +            exit(EXIT_SUCCESS);
>           case 'd':
>               daemonize = 1;
>               break;
>           case 'b': {
>               if (is_help_option(optarg)) {
>                   qmp_for_each_command(ga_print_cmd, NULL);
> -                return 0;
> +                exit(EXIT_SUCCESS);
>               }
>               for (j = 0, i = 0, len = strlen(optarg); i < len; i++) {
>                   if (optarg[i] == ',') {
> @@ -1027,36 +1027,36 @@ int main(int argc, char **argv)
>                                     NULL :
>                                     state_dir;
>                   if (ga_install_vss_provider()) {
> -                    return EXIT_FAILURE;
> +                    exit(EXIT_FAILURE);
>                   }
>                   if (ga_install_service(path, log_filepath, fixed_state_dir)) {
> -                    return EXIT_FAILURE;
> +                    exit(EXIT_FAILURE);
>                   }
> -                return 0;
> +                exit(EXIT_SUCCESS);
>               } else if (strcmp(service, "uninstall") == 0) {
>                   ga_uninstall_vss_provider();
> -                return ga_uninstall_service();
> +                exit(ga_uninstall_service());
>               } else if (strcmp(service, "vss-install") == 0) {
>                   if (ga_install_vss_provider()) {
> -                    return EXIT_FAILURE;
> +                    exit(EXIT_FAILURE);
>                   }
> -                return EXIT_SUCCESS;
> +                exit(EXIT_SUCCESS);
>               } else if (strcmp(service, "vss-uninstall") == 0) {
>                   ga_uninstall_vss_provider();
> -                return EXIT_SUCCESS;
> +                exit(EXIT_SUCCESS);
>               } else {
>                   printf("Unknown service command.\n");
> -                return EXIT_FAILURE;
> +                exit(EXIT_FAILURE);
>               }
>               break;
>   #endif
>           case 'h':
>               usage(argv[0]);
> -            return 0;
> +            exit(EXIT_SUCCESS);
>           case '?':
>               g_print("Unknown option, try '%s --help' for more information.\n",
>                       argv[0]);
> -            return EXIT_FAILURE;
> +            exit(EXIT_FAILURE);
>           }
>       }
>   
Fairly speaking the approach is not very clean as
it can be. exit() in the middle if the program is
almost the worst thing to be done as it
introduces various leaks. Anyway, pls conside
these words as world of old slowpoke. Technically
the patch is correct thus

Reviewed-by: Denis V. Lunev <den@openvz.org>

except typos above.