From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=40549 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1OKQgq-0000FH-7H for qemu-devel@nongnu.org; Fri, 04 Jun 2010 02:47:37 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1OKQgo-0005YX-Vp for qemu-devel@nongnu.org; Fri, 04 Jun 2010 02:47:36 -0400 Received: from mx1.redhat.com ([209.132.183.28]:49451) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1OKQgo-0005YI-O9 for qemu-devel@nongnu.org; Fri, 04 Jun 2010 02:47:34 -0400 Message-ID: <4C08A183.2070704@redhat.com> Date: Fri, 04 Jun 2010 08:47:31 +0200 From: Jes Sorensen 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> <4C081774.1020300@twiddle.net> In-Reply-To: <4C081774.1020300@twiddle.net> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Richard Henderson Cc: qemu-devel@nongnu.org On 06/03/10 22:58, Richard Henderson wrote: > 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... > The problem is that it requires qemu-options.h to be included, which isn't included per default for all the files. If I put it into sysemu.h at least it's going to require making every .c file build with those flags. I agree it's gross, but I am not sure what would be a better solution. >> + 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? I was trying to make it forward looking, but yeah we can just kill that. Cheers, Jes