From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47735) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZhHj2-0000Zr-BX for qemu-devel@nongnu.org; Wed, 30 Sep 2015 09:47:17 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZhHiy-00054g-8e for qemu-devel@nongnu.org; Wed, 30 Sep 2015 09:47:16 -0400 References: <1443184788-18859-1-git-send-email-afaerber@suse.de> <1443184788-18859-2-git-send-email-afaerber@suse.de> <878u7o2gfm.fsf@blackfin.pond.sub.org> From: Eric Blake Message-ID: <560BE7D9.9000006@redhat.com> Date: Wed, 30 Sep 2015 07:47:05 -0600 MIME-Version: 1.0 In-Reply-To: <878u7o2gfm.fsf@blackfin.pond.sub.org> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="diHSXC3191mHldiackfJpNLlRGonAXx4C" Subject: Re: [Qemu-devel] [PATCH 1/7] string-input-visitor: Fix uint64 parsing List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster , =?UTF-8?Q?Andreas_F=c3=a4rber?= Cc: qemu-devel@nongnu.org, qemu-stable@nongnu.org, Michael Roth , Bruce Rogers , Lin Ma , Paolo Bonzini This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --diHSXC3191mHldiackfJpNLlRGonAXx4C Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 09/30/2015 07:19 AM, Markus Armbruster wrote: >=20 > The (essentially undocumented) Visitor abstraction has the following > methods for integers: I proposed documentation at: https://lists.gnu.org/archive/html/qemu-devel/2015-09/msg05434.html >=20 > * Mandatory: type_int() >=20 > Interface uses int64_t for the value. The implementation should > ensure it fits into int64_t. >=20 > * Optional: type_int{8,16,32}() >=20 > These use int{8,16,32}_t for the value. >=20 > If present, it should ensure the value fits into the data type. >=20 > If missing, the core falls back to type_int() plus appropriate range > checking. No one implements them. In fact, as part of preparing my documentation, I actually proposed simplifying the visitor callback interface to drop th= em: https://lists.gnu.org/archive/html/qemu-devel/2015-09/msg05432.html >=20 > * Optional: type_int64() >=20 > Same interface as type_int(). >=20 > If present, it should ensure the value fits into int64_t. >=20 > If missing, the core falls back to type_int(). >=20 > Aside: setting type_int64() would be useful only when you want to > distinguish QAPI types int and int64. So far, nobody does. In fact,= > nobody uses QAPI type int64! I'm tempted to define QAPI type int as = a > mere alias for int64 and drop the redundant stuff. Already part of my proposal. >=20 > * Optional: type_uint{8,16,32}() >=20 > These use uint{8,16,32}_t for the value. >=20 > If present, it should ensure the value fits into the data type. >=20 > If missing, the core falls back to type_int() plus appropriate range > checking. Also unused, and simplified above. >=20 > * Optional: type_uint64() >=20 > Now it gets interesting. Interface uses uint64_t for the value. >=20 > If present, it should ensure the value fits into uint64_t. >=20 > If missing, the core falls back to type_int(). No range checking. I= f > type_int() performs range checking as it should, then uint64_t values= > not representable in int64_t get rejected (wrong), and negative value= s > representable in int64_t get cast to uint64_t (also wrong). >=20 > I think we need to make type_uint64() mandatory, and drop the > fallback. Probably a good idea, although not done in my proposed patches. >=20 > * Optional: type_size() >=20 > Same interface as type_uint64(). >=20 > If present, it should ensure the value fits into uint64_t. >=20 > If missing, the core first tries falling back to type_uint64() and > then to type_int(). Falling back to type_int() is as wrong here as i= t > is in type_uint64(). Provided by the QemuOpts parser to allow '1k' to mean 1024, and so on. >=20 >> As a bug fix, ignore warnings about preference of qemu_strto[u]ll(). >=20 > I'm not sure I get this sentence. >=20 >> Cc: qemu-stable@nongnu.org >> Signed-off-by: Andreas F=C3=A4rber >=20 > On the actual patch, I have nothing to add over Eric's review right now= =2E >=20 --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --diHSXC3191mHldiackfJpNLlRGonAXx4C 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/ iQEcBAEBCAAGBQJWC+fZAAoJEKeha0olJ0NqhhcH/jd7+YXHGaWsVsDgxJhP7kzA 7m+f45j4mRrIsduHA4pOHJ+yrzod8sXuJxPfP7P1M8yRwG7lQtpn14W+XwS0knEz r9/MozTieAsYlfEAWTx4mqvxKBUFUy/aFTcN/cNC7AOT9vpJdZeqm/5QbZgye+Jq gzIA23aN+VcYJVwwChJciaZ/GVYitBfKpeVBRwKLewckCnYdBGLnnofzGUBQ3piZ 7fpJVg/803RrRa/ZGkZe8Si4VTBbvNCwEX+xOTLEwUr9NjjzfJw31iQmVhL1hohu wHIKZLqEN5fhq8FCO8DvOLXdkz+mx8Vq7LM7zD+XoZFB8276lvJsPGQDZ5Mo4TU= =ucbF -----END PGP SIGNATURE----- --diHSXC3191mHldiackfJpNLlRGonAXx4C--