From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60962) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1avqh2-0007G4-Sx for qemu-devel@nongnu.org; Thu, 28 Apr 2016 14:29:41 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1avqh1-0004fk-VE for qemu-devel@nongnu.org; Thu, 28 Apr 2016 14:29:40 -0400 From: Markus Armbruster References: <1461843366-27217-1-git-send-email-kwolf@redhat.com> <1461843366-27217-2-git-send-email-kwolf@redhat.com> Date: Thu, 28 Apr 2016 20:29:32 +0200 In-Reply-To: <1461843366-27217-2-git-send-email-kwolf@redhat.com> (Kevin Wolf's message of "Thu, 28 Apr 2016 13:36:05 +0200") Message-ID: <871t5pd1tf.fsf@dusky.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain Subject: Re: [Qemu-devel] [PATCH v2 1/2] vvfat: Fix volume name assertion List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf Cc: qemu-block@nongnu.org, w.bumiller@proxmox.com, qemu-devel@nongnu.org, mreitz@redhat.com, stefanha@redhat.com Kevin Wolf writes: > Commit d5941dd made the volume name configurable, but it didn't consider > 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. > > This fixes the assertion to compare with the configured volume name > instead of a literal string. > > Cc: qemu-stable@nongnu.org > Signed-off-by: Kevin Wolf > --- > block/vvfat.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/block/vvfat.c b/block/vvfat.c > index 6b85314..ff3df35 100644 > --- a/block/vvfat.c > +++ b/block/vvfat.c > @@ -2283,12 +2283,17 @@ DLOG(fprintf(stderr, "commit_direntries for %s, parent_mapping_index %d\n", mapp > factor * (old_cluster_count - new_cluster_count)); > > for (c = first_cluster; !fat_eof(s, c); c = modified_fat_get(s, c)) { > + direntry_t *first_direntry; > void* direntry = array_get(&(s->directory), current_dir_index); > int ret = vvfat_read(s->bs, cluster2sector(s, c), direntry, > s->sectors_per_cluster); > if (ret) > return ret; > - assert(!strncmp(s->directory.pointer, "QEMU", 4)); Typing all of "QEMU VVAT" a third time was clearly too much. > + > + /* The first directory entry on the filesystem is the volume name */ > + first_direntry = (direntry_t*) s->directory.pointer; I'd ask to correct the spacing to (direntry_t *)s if the spacing wasn't similarly off all over this file. > + assert(!memcmp(first_direntry->name, s->volume_label, 11)); > + > current_dir_index += factor; > } Might want to to assert is_volume_label(), too. But even if you want to, let's not delay the fix for that. Reviewed-by: Markus Armbruster