From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:36367) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UzXHy-0008VX-MH for qemu-devel@nongnu.org; Wed, 17 Jul 2013 15:21:29 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UzXHx-00048R-9F for qemu-devel@nongnu.org; Wed, 17 Jul 2013 15:21:26 -0400 Received: from mx1.redhat.com ([209.132.183.28]:38741) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UzXHx-00048M-1o for qemu-devel@nongnu.org; Wed, 17 Jul 2013 15:21:25 -0400 Message-ID: <51E6EEB2.4060603@redhat.com> Date: Wed, 17 Jul 2013 13:21:22 -0600 From: Eric Blake MIME-Version: 1.0 References: <1374081369-1511-1-git-send-email-armbru@redhat.com> <1374081369-1511-5-git-send-email-armbru@redhat.com> In-Reply-To: <1374081369-1511-5-git-send-email-armbru@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="----enig2BUCTBPHPSWKSHKWTKCUI" Subject: Re: [Qemu-devel] [PATCH 4/7] smbios: Make multiple -smbios type= accumulate sanely List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: aliguori@us.ibm.com, lersek@redhat.com, qemu-devel@nongnu.org, ehabkost@redhat.com This is an OpenPGP/MIME signed message (RFC 4880 and 3156) ------enig2BUCTBPHPSWKSHKWTKCUI Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 07/17/2013 11:16 AM, Markus Armbruster wrote: > Currently, -smbios type=3DT,NAME=3DVAL,... adds one field (T,NAME) with= > value VAL to fw_cfg for each unique NAME. If NAME occurs multiple > times, the last one's VAL is used (before the QemuOpts conversion, the > first one was used). >=20 > Multiple -smbios can add multiple fields with the same (T, NAME). > SeaBIOS reads all of them from fw_cfg, but uses only the first field > (T, NAME). The others are ignored. >=20 > "First one wins, subsequent ones get ignored silently" isn't nice. We > commonly let the last option win. Useful, because it lets you > -readconfig first, then selectively override with command line > options. >=20 > Clean up -smbios to work the common way. Accumulate the settings, > with later ones overwriting earlier ones. Put the result into fw_cfg > (no more useless duplicates). >=20 > Bonus cleanup: qemu_uuid_parse() no longer sets SMBIOS system uuid by > side effect. >=20 > Signed-off-by: Markus Armbruster > --- > } > - val =3D qemu_opt_get(opts, "release"); > - if (val) { > - if (sscanf(val, "%hhu.%hhu", &major, &minor) !=3D 2) { > - error_report("Invalid release"); > - exit(1); > @@ -348,7 +353,18 @@ void smbios_entry_add(QemuOpts *opts) > error_report("%s", error_get_pretty(local_err)); > exit(1); > } > - smbios_build_type_0_fields(opts); > + save_opt(&type0.vendor, opts, "vendor"); > + save_opt(&type0.version, opts, "version"); > + save_opt(&type0.date, opts, "date"); > + > + val =3D qemu_opt_get(opts, "release"); > + if (val) { > + if (sscanf(val, "%hhu.%hhu", &type0.major, &type0.mino= r) !=3D 2) { sscanf() is lousy at detecting overflow. Since this is just code motion, you can get away without changing it now, but it would be nice if you at least considered switching this to use [a sane wrapper around] strtol() for proper overflow detection of user input at some point in your refactoring. > + val =3D qemu_opt_get(opts, "uuid"); > + if (val) { > + if (qemu_uuid_parse(val, qemu_uuid) !=3D 0) { > + error_report("Invalid UUID"); > + exit(1); My comments from earlier in the series about preferring exit(EXIT_FAILURE) apply here as well. Neither issue is a show-stopper, so Reviewed-by: Eric Blake --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org ------enig2BUCTBPHPSWKSHKWTKCUI Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.13 (GNU/Linux) Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJR5u6yAAoJEKeha0olJ0NqvEIH/ijuu0yifW0RBO+PveYWS0tY 5fIivPRQtt9WbvSCwOpH6dDhEFfFKzuHBZCNcpxhkcDLucf/FVg1I+06VSgdkc/b 7bSIfDhvkRNCD5a/G9l7dAUTgiN7zQE+7HLLjuTySBw0AWhpe4yX4DIJAE2lDoVZ k7gCxWbOWvjk1+OuCk7Bz1QRzUtlqYqj5up1I7/o202bFldzr19B+C3v8lmOF5ho OLyWhFWfRJMcMV1NNFx+nXOIRiSSWsECoFWacpIiYGUpWVJ/r8VpJ/qUKvn2YMOv N+cZ+ALG9/JsN7nSsCvfcuVsvmToHkbiRax2ZrVQXE1keDQWfLG8gBU4TAstUEA= =Sp3d -----END PGP SIGNATURE----- ------enig2BUCTBPHPSWKSHKWTKCUI--