| Message ID | 1461759883-8589-2-git-send-email-kwolf@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 27.04.2016 14:24, Kevin Wolf wrote: > 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 <kwolf@redhat.com> > --- > block/vvfat.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > 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 name */ > + direntry_t *first_direntry = 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 += factor; > } > >
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 name */ + direntry_t *first_direntry = s->directory.pointer; + assert(!memcmp(first_direntry->name, s->volume_label, 11)); + current_dir_index += factor; }
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 <kwolf@redhat.com> --- block/vvfat.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)