From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34943) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Wy0dF-0007XB-4F for qemu-devel@nongnu.org; Fri, 20 Jun 2014 11:21:42 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Wy0dA-0007ZH-6w for qemu-devel@nongnu.org; Fri, 20 Jun 2014 11:21:37 -0400 Received: from mx1.redhat.com ([209.132.183.28]:35203) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Wy0d9-0007Z8-Uh for qemu-devel@nongnu.org; Fri, 20 Jun 2014 11:21:32 -0400 Message-ID: <53A45171.1080406@redhat.com> Date: Fri, 20 Jun 2014 09:21:21 -0600 From: Eric Blake MIME-Version: 1.0 References: <20140605145714.10787.97053.stgit@hds.com> <20140605145734.10787.28959.stgit@hds.com> <53A354D5.1030506@redhat.com> In-Reply-To: Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="Q7L1bavFKVIwa9Tbpb8hoG1QgmJ5s6jCV" Subject: Re: [Qemu-devel] [PATCH v4 2/2] qga: Add guest-get-fsinfo command List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Tomoki Sekiyama , "qemu-devel@nongnu.org" Cc: Mitsuhiro Tanino , "mdroth@linux.vnet.ibm.com" This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --Q7L1bavFKVIwa9Tbpb8hoG1QgmJ5s6jCV Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 06/19/2014 06:34 PM, Tomoki Sekiyama wrote: >>> + } >>> + if (S_ISBLK(st.st_mode)) { >>> + *devmajor =3D major(st.st_rdev); >>> + *devminor =3D minor(st.st_rdev); >> >> major() and minor() are not POSIX functions. While they work on Linux= , >> and appear to have BSD origins, I still wonder if you need to be more >> careful on guarding their use. >=20 > This function is guarded by '#if defined(__linux__)' (and also > '#if defined(CONFIG_FSFREEZE) || defined(CONFIG_FSTRIM)' ), > so I believe it is ok here. I take it the function gracefully fails on non-Linux. But that's not very nice to management functions - they learn that the function exists, but have to call it to see if it actually works. It might be nicer to conditionally expose the command only if it will work, so that querying the list of commands makes it obvious whether the agent supports subset freezing. >>> + while (getline(&line, &n, fp) !=3D -1) { >>> + ret =3D sscanf(line, >>> + "%*u %*u %u:%u %*s %n%*s%n %*s %*s %*s %n%*s%n >>> %n%*s%n%c", >>> + &devmajor, &devminor, &dir_s, &dir_e, &type_s, >>> &type_e, >> >> I'm not a huge fan of sscanf("%u") - it has undefined behavior on >> integer overflow. But the alternative of avoiding sscanf and >> open-coding your parser is so much bulkier, that I tend to look the >> other way. >=20 > Hmm, '%*u' can be simply replaced by '%*s' then. > For '%u:%u', maybe we can catch this part with '%s' or '%n%*s%n' > and convert them into integers later by strtoul(). > Does it sound good to you? As I said, the cost of properly parsing a row of integers explodes into so much more code that I'm just fine looking the other way if you want to use sscanf for compactness - after all, this is a kernel file we're supposed to be reading, and if that interface has been corrupted to give us bogus data that doesn't parse correctly, then we're probably already hurting in other ways. --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --Q7L1bavFKVIwa9Tbpb8hoG1QgmJ5s6jCV 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 Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJTpFFxAAoJEKeha0olJ0NqKW4IAJdjCSbV7B/D9bXvLbgewTun gIZ0638LvQh9GASRl7QJBISsdLf5rWA5IbDRLL8rgayQ9E6jFOTgpv4c6E50FCjd Rgy2it2C9rtLvZTTaP8I5krncq8JZz9QzDKkcbGdW3PWO6MoQWISyc6PljpjhejY JnrdBXDJHlFQ+sRtYKlZmgu2eXFZTdl4XmBfavlgzwoy7vAkkU/5WyerjbZTnylL Nj26svi5TzcJ3cgFf1kM8oChLqDQdd4JO7mhXETk7LVbcUZeWnJ4dgtZj51EShfT wfKexLghVTtJPDa/sYD1OheRbaNqOPPVEeoE9QbXe4P/rbzViGVI4ptmpKXyXWY= =Bfha -----END PGP SIGNATURE----- --Q7L1bavFKVIwa9Tbpb8hoG1QgmJ5s6jCV--