From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60494) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1V4BLz-0006eN-Ra for qemu-devel@nongnu.org; Tue, 30 Jul 2013 10:56:52 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1V4BLv-0004Ka-68 for qemu-devel@nongnu.org; Tue, 30 Jul 2013 10:56:47 -0400 Received: from mx1.redhat.com ([209.132.183.28]:44329) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1V4BLu-0004KU-Uf for qemu-devel@nongnu.org; Tue, 30 Jul 2013 10:56:43 -0400 Message-ID: <51F7D428.5090809@redhat.com> Date: Tue, 30 Jul 2013 16:56:40 +0200 From: Michal Novotny MIME-Version: 1.0 References: <5ce704edab8951c4e244d1e33e85d384dabb368e.1375194476.git.minovotn@redhat.com> In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] vl.c: Output error on invalid machine type provided List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Maydell Cc: qemu-devel@nongnu.org On 07/30/2013 04:46 PM, Peter Maydell wrote: > On 30 July 2013 15:28, Michal Novotny wrote: >> Output error message when user provides the invalid machine type >> on the command line. This also saves time to find what issue is >> when you upgrade from one version of qemu to another version that >> doesn't support required machine type any longer. >> >> Michal >> >> Signed-off-by: Michal Novotny >> --- >> vl.c | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/vl.c b/vl.c >> index 25b8f2f..4455b26 100644 >> --- a/vl.c >> +++ b/vl.c >> @@ -2671,6 +2671,10 @@ static QEMUMachine *machine_parse(const char *name) >> if (machine) { >> return machine; >> } >> + >> + if (*name != '0') >> + printf("Error: Unsupported machine type '%s'\n", name); >> + > Hi; thanks for the patch. I think the idea is a good > one but there are some minor issues with the implementation: > > This now causes "-M help" to print a spurious line > "Error: Unsupported machine type 'help'". > > You need braces around if statements, even one-liners. > (scripts/checkpatch.pl may help with this kind of thing.) > > This function can be called with name being NULL but > you try to dereference it. > > My suggestion is that your condition should be > if (name && !is_help_option(name)) { > ... > } Hi Peter, thanks a lot for your reply. I rewrote the patch and checked it using scripts/checkpatch.pl. It's already sent as v2 to the list. Thanks a lot for your feedback, Michal -- Michal Novotny , RHCE, Red Hat Virtualization | libvirt-php bindings | php-virt-control.org