From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53047) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WNTnS-0006fd-1V for qemu-devel@nongnu.org; Tue, 11 Mar 2014 17:01:15 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WNTnL-0000Ps-P6 for qemu-devel@nongnu.org; Tue, 11 Mar 2014 17:01:09 -0400 Received: from mail-ob0-f175.google.com ([209.85.214.175]:39234) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WNTnL-0000PQ-LR for qemu-devel@nongnu.org; Tue, 11 Mar 2014 17:01:03 -0400 Received: by mail-ob0-f175.google.com with SMTP id uy5so9113836obc.34 for ; Tue, 11 Mar 2014 14:01:02 -0700 (PDT) Date: Tue, 11 Mar 2014 21:00:04 +0000 From: Leandro Dorileo Message-ID: <20140311210004.GA23555@dorilex> References: <1394436721-21812-1-git-send-email-cyliu@suse.com> <1394436721-21812-4-git-send-email-cyliu@suse.com> <531E20AB.3090409@redhat.com> <531E2CC0.8080406@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Subject: Re: [Qemu-devel] [PATCH v22 03/25] improve some functions in qemu-option.c List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Chunyan Liu Cc: Kevin Wolf , Dong Xu Wang , qemu-devel@nongnu.org, stefanha@redhat.com Hi, On Tue, Mar 11, 2014 at 03:26:51PM +0800, Chunyan Liu wrote: > 2014-03-11 5:21 GMT+08:00 Eric Blake : > > > On 03/10/2014 02:29 PM, Eric Blake wrote: > > > > >> + opt = qemu_opt_find(opts, name); > > >> + if (opt) { > > >> + g_free((char *)opt->str); > > > > > > ...which means the cast is pointless here. > > > > > > Hmm. This means that you are giving opt_set() the behavior of 'last > > > version wins', by silently overwriting earlier versions. If I'm > > > understanding the existing code correctly, the previous behavior was > > > that calling opt_set twice in a row on the same name would inject BOTH > > > names into 'opts', but then subsequent lookups on opts would find the > > > FIRST hit. Doesn't that mean this is a semantic change: > > > > > > qemu -opt key=value1,key=value2 > > > > > > would previously set key to value1, but now sets key to value2. > > > > I've played with this a bit more, and now am more confused. QemuOpts is > > a LOT to comprehend. > > > > Pre-patch, 'qemu-system-x86_64 -nodefaults -machine > > type=none,type-noone' displayed a help message about unknown machine > > type "noone", while swapping type=noone,type=none proceeded with the > > 'none' type. So the last version silently won, which was not the > > behavior I had predicted. > > > > Post-patch, I get a compilation error (so how did you test your patch?): > > > > Mostly tested ./qemu-img commands where QEMUOptionParameter is used. > I really didn't think of test QemuOpts fully, and about the test suite, I > have no full > knowledge about how many things need to be tested, how many things need to > be > covered? The testsuite should test the QemuOpts implementation, not the current users. Regards... -- Leandro Dorileo