From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53558) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZaNZu-0002bS-LX for qemu-devel@nongnu.org; Fri, 11 Sep 2015 08:37:19 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZaNZp-0007nt-Vv for qemu-devel@nongnu.org; Fri, 11 Sep 2015 08:37:18 -0400 Received: from mx1.redhat.com ([209.132.183.28]:53359) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZaNZp-0007mu-N3 for qemu-devel@nongnu.org; Fri, 11 Sep 2015 08:37:13 -0400 References: <1441973427-8897-1-git-send-email-berrange@redhat.com> <1441973427-8897-2-git-send-email-berrange@redhat.com> From: Eric Blake Message-ID: <55F2CAF4.7090807@redhat.com> Date: Fri, 11 Sep 2015 06:37:08 -0600 MIME-Version: 1.0 In-Reply-To: <1441973427-8897-2-git-send-email-berrange@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="eCkeCMT48MUt5G5j6h0tuaMgw9eF9hSmW" Subject: Re: [Qemu-devel] [PATCH PULL 01/11] qapi: allow override of default enum prefix naming List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Daniel P. Berrange" , qemu-devel@nongnu.org Cc: Peter Maydell This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --eCkeCMT48MUt5G5j6h0tuaMgw9eF9hSmW Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 09/11/2015 06:10 AM, Daniel P. Berrange wrote: > The camel_to_upper() method applies some heuristics to turn > a mixed case type name into an all-uppercase name. This is > used for example, to generate enum constant name prefixes. >=20 > The heuristics don't also generate a satisfactory name > though. eg >=20 > { 'enum': 'QCryptoTLSCredsEndpoint', > 'data': ['client', 'server']} >=20 > Results in Q_CRYPTOTLS_CREDS_ENDPOINT_CLIENT. This has > an undesirable _ after the initial Q and is missing an > _ between the CRYPTO & TLS strings. >=20 > Rather than try to add more and more heuristics to try > to cope with this, simply allow the QAPI schema to > specify the desired enum constant prefix explicitly. >=20 > eg >=20 > { 'enum': 'QCryptoTLSCredsEndpoint', > 'prefix': 'QCRYPTO_TLS_CREDS_ENDPOINT', > 'data': ['client', 'server']} >=20 > Now gives the QCRYPTO_TLS_CREDS_ENDPOINT_CLIENT name. >=20 > Signed-off-by: Daniel P. Berrange > --- Missing Reviewed-by tags. Probably because I haven't had a chance to look at v6 yet, and didn't leave R-b on v5. Paolo gave blanket review, but I'm still interested in reviewing. In particular, > +++ b/docs/qapi-code-gen.txt > @@ -236,6 +236,7 @@ both fields like this: > =3D=3D=3D Enumeration types =3D=3D=3D > =20 > Usage: { 'enum': STRING, 'data': ARRAY-OF-STRING } > + { 'enum': STRING, 'prefix': STRING, 'data': ARRAY-OF-STRING } s/'prefix'/'*prefix'/ to mark that prefix is optional. > +++ b/scripts/qapi-types.py > @@ -348,9 +348,11 @@ for expr in exprs: > if expr.has_key('struct'): > ret +=3D generate_fwd_struct(expr['struct']) > elif expr.has_key('enum'): > - ret +=3D generate_enum(expr['enum'], expr['data']) > + ret +=3D generate_enum(expr['enum'], expr['data'], > + expr.get('prefix')) > ret +=3D generate_fwd_enum_struct(expr['enum']) > - fdef.write(generate_enum_lookup(expr['enum'], expr['data'])) > + fdef.write(generate_enum_lookup(expr['enum'], expr['data'], > + expr.get('prefix'))) > elif expr.has_key('union'): And this probably introduces merge conflicts with Markus' introspection work. I'm not sure which series should go in first, but Markus' has certainly been on the list longer and has higher complexity. > +++ b/tests/Makefile > @@ -221,7 +221,8 @@ check-qapi-schema-y :=3D $(addprefix tests/qapi-sch= ema/, \ > comments.json empty.json enum-empty.json enum-missing-data.json \ > enum-wrong-data.json enum-int-member.json enum-dict-member.json \ > enum-clash-member.json enum-max-member.json enum-union-clash.json \ > - enum-bad-name.json funny-char.json indented-expr.json \ > + enum-bad-name.json enum-bad-prefix.json \ > + funny-char.json indented-expr.json \ > missing-type.json bad-ident.json ident-with-escape.json \ > escape-outside-string.json unknown-escape.json \ > escape-too-short.json escape-too-big.json unicode-str.json \ > @@ -520,7 +521,7 @@ $(patsubst %, check-%, $(check-qapi-schema-y)): che= ck-%.json: $(SRC_PATH)/%.json > " TEST $*.out") > @diff -q $(SRC_PATH)/$*.out $*.test.out > @# Sanitize error messages (make them independent of build directory)= > - @perl -p -e 's|\Q$(SRC_PATH)\E/||g' $*.test.err | diff -q $(SRC_PATH)= /$*.err - > + @perl -p -e 's|\Q$(SRC_PATH)\E/||g' $*.test.err | diff $(SRC_PATH)/$= *.err - This hunk feels like it might need to be a separate patch, or at least explained in the commit message. --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --eCkeCMT48MUt5G5j6h0tuaMgw9eF9hSmW Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJV8sr0AAoJEKeha0olJ0NqYxUH/0weXzU3H6W6n+IcK3Z7EOct jePEnbV3GNKLaGPTFM2zXXWjpas3izgJ3KiJGOl3at3w0OvBOjTYtkng643/8cVN o9vUyl38YNKD4AinH1CNoWE7nYmWsN/ivt6BPMzad9TkztQtF3ouWDUL29cVMybx NFq9opNH6zHvy+15BOWjnmu25RRU92aAG98v71TTLTW30fb/NEFc4t4AvLNkSIeM KrQfUYLXoyM+hS4yZg2ZY2UhehCP6Ad19SUdtsQ/73ApSCwJIUMrTd3w/C9zVhAY bdfYxqL/G+lnht1RsZqNXRTYb0Fl/duIijzEkaX7497JVPZN1chrNVhRVNezTFc= =Fj3m -----END PGP SIGNATURE----- --eCkeCMT48MUt5G5j6h0tuaMgw9eF9hSmW--