From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:52699) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WHs3V-0008D5-Ar for qemu-devel@nongnu.org; Mon, 24 Feb 2014 04:42:39 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WHs3P-0004I5-B2 for qemu-devel@nongnu.org; Mon, 24 Feb 2014 04:42:33 -0500 Received: from mx1.redhat.com ([209.132.183.28]:3439) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WHs3P-0004Hv-3U for qemu-devel@nongnu.org; Mon, 24 Feb 2014 04:42:27 -0500 Received: from int-mx02.intmail.prod.int.phx2.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s1O9gPiV024992 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Mon, 24 Feb 2014 04:42:25 -0500 Date: Mon, 24 Feb 2014 10:42:26 +0100 From: Kevin Wolf Message-ID: <20140224094226.GC3775@dhcp-200-207.str.redhat.com> References: <1392996248-26781-1-git-send-email-kwolf@redhat.com> <1392996248-26781-2-git-send-email-kwolf@redhat.com> <5307B593.7020801@redhat.com> <20140221205755.GD3346@dhcp-200-207.str.redhat.com> <87bnxw6gws.fsf@blackfin.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <87bnxw6gws.fsf@blackfin.pond.sub.org> Subject: Re: [Qemu-devel] [PATCH v3 1/6] qemu-option: has_help_option() and is_valid_option_list() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: jcody@redhat.com, qemu-devel@nongnu.org, stefanha@redhat.com Am 24.02.2014 um 09:42 hat Markus Armbruster geschrieben: > Kevin Wolf writes: > > > Am 21.02.2014 um 21:22 hat Eric Blake geschrieben: > >> On 02/21/2014 08:24 AM, Kevin Wolf wrote: > >> > has_help_option() checks if any help option ('help' or '?') occurs > >> > anywhere in an option string, so that things like 'cluster_size=4k,help' > >> > are recognised. > >> > > >> > is_valid_option_list() ensures that the option list doesn't have options > >> > with leading commas or trailing unescaped commas. > > Care to explain why these are bad? I dimly remember Eric and you > discussing it last week, but I sure won't next time I look at this > code... Because with them you can trick qemu-img into producing escaped commas when it tries to concatenate option strings ('-o backing_file=foo, -o help' results in '-o backing_file=foo,,help'). > >> Rejects empty options, but also rejects values beginning with a comma. > >> But we have legacy users that accept implicitly named first options (see > >> opts_do_parse()). For example, this is a valid command line (albeit one > >> that prints a list of valid machines): > >> > >> qemu-kvm -machine ,,blah > >> > >> as shorthand for > >> > >> qemu-kvm -machine type=,,blah > >> > >> and where *buf would indeed be validly ','. > > > > Right, but I can't allow this without allowing '-o ,,' which breaks the > > real use case. So the lesson is that you can concatenate option strings > > and use implicit option names at the same time. > > Another wart on the freakishly complex QemuOpts. Rather what you get for operating on strings instead of directly going to QemuOpts. We're parsing the option strings in the wrong place and qemu-img shouldn't be concatenating option strings. But it takes a larger cleanup until we can have that and I didn't want to block this improvement on infrastructure work. Kevin