From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from [140.186.70.92] (port=55540 helo=eggs.gnu.org)
	by lists.gnu.org with esmtp (Exim 4.43) id 1OKHjr-0007gc-Nu
	for qemu-devel@nongnu.org; Thu, 03 Jun 2010 17:14:09 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69)
	(envelope-from <rth@twiddle.net>) id 1OKHUk-0003fP-2a
	for qemu-devel@nongnu.org; Thu, 03 Jun 2010 16:58:31 -0400
Received: from are.twiddle.net ([75.149.56.221]:36833)
	by eggs.gnu.org with esmtp (Exim 4.69)
	(envelope-from <rth@twiddle.net>) id 1OKHUj-0003f4-So
	for qemu-devel@nongnu.org; Thu, 03 Jun 2010 16:58:30 -0400
Message-ID: <4C081774.1020300@twiddle.net>
Date: Thu, 03 Jun 2010 13:58:28 -0700
From: Richard Henderson <rth@twiddle.net>
MIME-Version: 1.0
Subject: Re: [Qemu-devel] [PATCH 10/16] Introduce OS specific cmdline argument
	handling and move SMB arg to os-posix.c
References: <1275583692-11678-1-git-send-email-Jes.Sorensen@redhat.com>
	<1275583692-11678-11-git-send-email-Jes.Sorensen@redhat.com>
In-Reply-To: <1275583692-11678-11-git-send-email-Jes.Sorensen@redhat.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
List-Id: qemu-devel.nongnu.org
List-Unsubscribe: <http://lists.nongnu.org/mailman/listinfo/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: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Jes.Sorensen@redhat.com
Cc: qemu-devel@nongnu.org

On 06/03/2010 09:48 AM, Jes.Sorensen@redhat.com wrote:
> +/*
> + * Duplicate definition from vl.c to avoid messing up the entire build
> + */
> +enum {
> +#define DEF(option, opt_arg, opt_enum, opt_help, arch_mask)     \
> +    opt_enum,
> +#define DEFHEADING(text)
> +#include "qemu-options.h"
> +#undef DEF
> +#undef DEFHEADING
> +#undef GEN_DOCS
> +};

There's no header file you can put this in?  Or invent to put this in?
Cause this is really kinda gross...

> +
> +/*
> + * Parse OS specific command line options.
> + * return 0 if option handled, -1 otherwise
> + */
> +int os_parse_cmd_args(const QEMUOption *popt, const char *optarg)
> +{
> +    int ret = 0;
> +    switch (popt->index) {
> +#ifdef CONFIG_SLIRP
> +    case QEMU_OPTION_smb:
> +        if (net_slirp_smb(optarg) < 0)
> +            exit(1);
> +        break;
> +#endif
> +    default:
> +        ret = -1;
> +    }
> +    return ret;
> +}

Why have a return value at all...

> +            default:
> +                os_parse_cmd_args(popt, optarg);

... if you're going to ignore the results?


r~