From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:56743) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1avQ3R-0005UQ-1Y for qemu-devel@nongnu.org; Wed, 27 Apr 2016 10:03:04 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1avQ3Q-0005Jn-3a for qemu-devel@nongnu.org; Wed, 27 Apr 2016 10:03:00 -0400 References: <1461759883-8589-1-git-send-email-kwolf@redhat.com> <1461759883-8589-2-git-send-email-kwolf@redhat.com> From: Max Reitz Message-ID: <5720C68A.4090605@redhat.com> Date: Wed, 27 Apr 2016 16:02:50 +0200 MIME-Version: 1.0 In-Reply-To: <1461759883-8589-2-git-send-email-kwolf@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="EO7Nm5DboPo26DR0vAKl8LO4mT3QOfr6I" Subject: Re: [Qemu-devel] [PATCH 1/2] vvfat: Fix volume name assertion List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf , qemu-block@nongnu.org Cc: stefanha@redhat.com, w.bumiller@proxmox.com, eblake@redhat.com, qemu-devel@nongnu.org This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --EO7Nm5DboPo26DR0vAKl8LO4mT3QOfr6I Content-Type: multipart/mixed; boundary="jv9Vwl1oR9FPPljGWpHRB3w8h0PwT76mf" From: Max Reitz To: Kevin Wolf , qemu-block@nongnu.org Cc: stefanha@redhat.com, w.bumiller@proxmox.com, eblake@redhat.com, qemu-devel@nongnu.org Message-ID: <5720C68A.4090605@redhat.com> Subject: Re: [PATCH 1/2] vvfat: Fix volume name assertion References: <1461759883-8589-1-git-send-email-kwolf@redhat.com> <1461759883-8589-2-git-send-email-kwolf@redhat.com> In-Reply-To: <1461759883-8589-2-git-send-email-kwolf@redhat.com> --jv9Vwl1oR9FPPljGWpHRB3w8h0PwT76mf Content-Type: text/plain; charset=iso-8859-15 Content-Transfer-Encoding: quoted-printable On 27.04.2016 14:24, Kevin Wolf wrote: > Commit d5941dd made the volume name configurable, but it didn't conside= r > that the rw code compares the volume name string to assert that the > first directory entry is the volume name. This made vvfat crash in rw > mode. >=20 > This fixes the assertion to compare with the configured volume name > instead of a literal string. >=20 > Cc: qemu-stable@nongnu.org > Signed-off-by: Kevin Wolf > --- > block/vvfat.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) >=20 > diff --git a/block/vvfat.c b/block/vvfat.c > index 6b85314..f9d4e82 100644 > --- a/block/vvfat.c > +++ b/block/vvfat.c > @@ -2288,7 +2288,11 @@ DLOG(fprintf(stderr, "commit_direntries for %s, = parent_mapping_index %d\n", mapp > s->sectors_per_cluster); > if (ret) > return ret; > - assert(!strncmp(s->directory.pointer, "QEMU", 4)); > + > + /* The first directory entry on the filesystem is the volume n= ame */ > + direntry_t *first_direntry =3D s->directory.pointer; I am afraid this does not conform to the QEMU coding style. Declaration of variables ("first_direntry" in this case) needs to be done at the beginning of the block, and this block starts with the for () loop. Fortunately, the vvfat code provides plenty of examples on how to do this right. I strongly support choosing the well-established style of creating a non-conditional block just around these three lines so that the declaration does not need to be moved. See the declaration of the direntry_t pointer "entry" in init_directories(), for instance. > + assert(!memcmp(first_direntry->name, s->volume_label, 11)); The code mentioned above also teaches us that we can use sizeof(first_direntry->name) instead of 11. However, I'll leave that up to you. Max > + > current_dir_index +=3D factor; > } > =20 >=20 --jv9Vwl1oR9FPPljGWpHRB3w8h0PwT76mf-- --EO7Nm5DboPo26DR0vAKl8LO4mT3QOfr6I 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 iQEcBAEBCAAGBQJXIMaLAAoJEDuxQgLoOKytxlIH/1kp+n7Hvns6UQUZl3kJINtN 3JaAhpTONDznASg7flmbJ5xJbkX0iem17N4Q3wMyqY954nIMnqTjd0j0IPeY1APz G7MmPldZlR2PBt07YEjj/5YFy2lZzfHsZSD+H6vxa9sPDrhvgf1uB8HCyVLTT7UU 5T1VJNrE+ViecPB3F1Ie07NZp+RcgrgyUnn0NmMODXrFussVVqLRM9dgolq42kHx qGiDP3RK096ALk2/gnrLPLenkK0JxLgk4qD6lZfX4G1iT/iYyWLF6h5GfZTaFYbU Kkw5ESyIneDCsxAKjgDQ+f8JyU/8HtVdt9brU3T/kL2cTuwhHoGXGmiu8c7r1PI= =8T5C -----END PGP SIGNATURE----- --EO7Nm5DboPo26DR0vAKl8LO4mT3QOfr6I--