Message ID | 1425023419-12244-7-git-send-email-arei.gonglei@huawei.com |
---|---|
State | New |
Headers | show |
27.02.2015 10:50, arei.gonglei@huawei.com wrote: > From: Gonglei <arei.gonglei@huawei.com> > > Signed-off-by: Gonglei <arei.gonglei@huawei.com> > --- > hw/9pfs/virtio-9p-local.c | 10 ++-------- > 1 file changed, 2 insertions(+), 8 deletions(-) > > diff --git a/hw/9pfs/virtio-9p-local.c b/hw/9pfs/virtio-9p-local.c > index a183eee..bcad4e0 100644 > --- a/hw/9pfs/virtio-9p-local.c > +++ b/hw/9pfs/virtio-9p-local.c > @@ -500,7 +500,6 @@ static int local_mknod(FsContext *fs_ctx, V9fsPath *dir_path, > buffer = rpath(fs_ctx, path); > err = mknod(buffer, SM_LOCAL_MODE_BITS|S_IFREG, 0); > if (err == -1) { > - g_free(buffer); > goto out; > } > err = local_set_xattr(buffer, credp); > @@ -513,7 +512,6 @@ static int local_mknod(FsContext *fs_ctx, V9fsPath *dir_path, > buffer = rpath(fs_ctx, path); > err = mknod(buffer, SM_LOCAL_MODE_BITS|S_IFREG, 0); > if (err == -1) { > - g_free(buffer); > goto out; > } > err = local_set_mapped_file_attr(fs_ctx, path, credp); > @@ -526,7 +524,6 @@ static int local_mknod(FsContext *fs_ctx, V9fsPath *dir_path, > buffer = rpath(fs_ctx, path); > err = mknod(buffer, credp->fc_mode, credp->fc_rdev); > if (err == -1) { > - g_free(buffer); > goto out; > } ... I wonder if we should change this code to drop repeated code blocks which changes only slightly by mknod parameters... > err = local_post_create_passthrough(fs_ctx, path, credp); > @@ -540,8 +537,8 @@ static int local_mknod(FsContext *fs_ctx, V9fsPath *dir_path, > err_end: > remove(buffer); > errno = serrno; > - g_free(buffer); > out: > + g_free(buffer); > v9fs_string_free(&fullname); > return err; > } ..and this goto jumping (here and in a few other places) is q bit disgusting too... ;) But this is the original code, the patch is still correct. Thanks, /mjt
27.02.2015 10:50, arei.gonglei@huawei.com wrote: > From: Gonglei <arei.gonglei@huawei.com> > > Signed-off-by: Gonglei <arei.gonglei@huawei.com> > --- > hw/9pfs/virtio-9p-local.c | 10 ++-------- > 1 file changed, 2 insertions(+), 8 deletions(-) > > diff --git a/hw/9pfs/virtio-9p-local.c b/hw/9pfs/virtio-9p-local.c > index a183eee..bcad4e0 100644 > --- a/hw/9pfs/virtio-9p-local.c > +++ b/hw/9pfs/virtio-9p-local.c > @@ -500,7 +500,6 @@ static int local_mknod(FsContext *fs_ctx, V9fsPath *dir_path, > buffer = rpath(fs_ctx, path); > err = mknod(buffer, SM_LOCAL_MODE_BITS|S_IFREG, 0); > if (err == -1) { > - g_free(buffer); > goto out; > } > err = local_set_xattr(buffer, credp); > @@ -513,7 +512,6 @@ static int local_mknod(FsContext *fs_ctx, V9fsPath *dir_path, > buffer = rpath(fs_ctx, path); > err = mknod(buffer, SM_LOCAL_MODE_BITS|S_IFREG, 0); > if (err == -1) { > - g_free(buffer); > goto out; > } > err = local_set_mapped_file_attr(fs_ctx, path, credp); > @@ -526,7 +524,6 @@ static int local_mknod(FsContext *fs_ctx, V9fsPath *dir_path, > buffer = rpath(fs_ctx, path); > err = mknod(buffer, credp->fc_mode, credp->fc_rdev); > if (err == -1) { > - g_free(buffer); > goto out; > } > err = local_post_create_passthrough(fs_ctx, path, credp); > @@ -540,8 +537,8 @@ static int local_mknod(FsContext *fs_ctx, V9fsPath *dir_path, > err_end: > remove(buffer); > errno = serrno; > - g_free(buffer); > out: > + g_free(buffer); > v9fs_string_free(&fullname); > return err; > } > @@ -676,7 +673,6 @@ static int local_open2(FsContext *fs_ctx, V9fsPath *dir_path, const char *name, > buffer = rpath(fs_ctx, path); > fd = open(buffer, flags, SM_LOCAL_MODE_BITS); > if (fd == -1) { > - g_free(buffer); > err = fd; > goto out; > } > @@ -691,7 +687,6 @@ static int local_open2(FsContext *fs_ctx, V9fsPath *dir_path, const char *name, > buffer = rpath(fs_ctx, path); > fd = open(buffer, flags, SM_LOCAL_MODE_BITS); > if (fd == -1) { > - g_free(buffer); > err = fd; > goto out; > } > @@ -707,7 +702,6 @@ static int local_open2(FsContext *fs_ctx, V9fsPath *dir_path, const char *name, > buffer = rpath(fs_ctx, path); > fd = open(buffer, flags, credp->fc_mode); > if (fd == -1) { > - g_free(buffer); > err = fd; > goto out; > } > @@ -725,8 +719,8 @@ err_end: > close(fd); > remove(buffer); > errno = serrno; > - g_free(buffer); > out: > + g_free(buffer); > v9fs_string_free(&fullname); > return err; > } This patch introduces a compiler warning, which is technically correct: hw/9pfs/virtio-9p-local.c: In function ‘local_open2’: hw/9pfs/virtio-9p-local.c:723:5: error: ‘buffer’ may be used uninitialized in this function [-Werror=maybe-uninitialized] g_free(buffer); ^ hw/9pfs/virtio-9p-local.c: In function ‘local_mknod’: hw/9pfs/virtio-9p-local.c:541:5: error: ‘buffer’ may be used uninitialized in this function [-Werror=maybe-uninitialized] g_free(buffer); ^ This is because `buffer' variable is initialized within one of the `if' branches but is left uninitialized if none of the conditions is true. In reality this can't happen because at least one condition is always true. It is a one more reason to rewrite this code like I mentioned in another mail instead of trying to fix random issues like this. I'll try to come up with a patch doing that shortly. Meanwhile I'll drop this patch. Thanks, /mjt
diff --git a/hw/9pfs/virtio-9p-local.c b/hw/9pfs/virtio-9p-local.c index a183eee..bcad4e0 100644 --- a/hw/9pfs/virtio-9p-local.c +++ b/hw/9pfs/virtio-9p-local.c @@ -500,7 +500,6 @@ static int local_mknod(FsContext *fs_ctx, V9fsPath *dir_path, buffer = rpath(fs_ctx, path); err = mknod(buffer, SM_LOCAL_MODE_BITS|S_IFREG, 0); if (err == -1) { - g_free(buffer); goto out; } err = local_set_xattr(buffer, credp); @@ -513,7 +512,6 @@ static int local_mknod(FsContext *fs_ctx, V9fsPath *dir_path, buffer = rpath(fs_ctx, path); err = mknod(buffer, SM_LOCAL_MODE_BITS|S_IFREG, 0); if (err == -1) { - g_free(buffer); goto out; } err = local_set_mapped_file_attr(fs_ctx, path, credp); @@ -526,7 +524,6 @@ static int local_mknod(FsContext *fs_ctx, V9fsPath *dir_path, buffer = rpath(fs_ctx, path); err = mknod(buffer, credp->fc_mode, credp->fc_rdev); if (err == -1) { - g_free(buffer); goto out; } err = local_post_create_passthrough(fs_ctx, path, credp); @@ -540,8 +537,8 @@ static int local_mknod(FsContext *fs_ctx, V9fsPath *dir_path, err_end: remove(buffer); errno = serrno; - g_free(buffer); out: + g_free(buffer); v9fs_string_free(&fullname); return err; } @@ -676,7 +673,6 @@ static int local_open2(FsContext *fs_ctx, V9fsPath *dir_path, const char *name, buffer = rpath(fs_ctx, path); fd = open(buffer, flags, SM_LOCAL_MODE_BITS); if (fd == -1) { - g_free(buffer); err = fd; goto out; } @@ -691,7 +687,6 @@ static int local_open2(FsContext *fs_ctx, V9fsPath *dir_path, const char *name, buffer = rpath(fs_ctx, path); fd = open(buffer, flags, SM_LOCAL_MODE_BITS); if (fd == -1) { - g_free(buffer); err = fd; goto out; } @@ -707,7 +702,6 @@ static int local_open2(FsContext *fs_ctx, V9fsPath *dir_path, const char *name, buffer = rpath(fs_ctx, path); fd = open(buffer, flags, credp->fc_mode); if (fd == -1) { - g_free(buffer); err = fd; goto out; } @@ -725,8 +719,8 @@ err_end: close(fd); remove(buffer); errno = serrno; - g_free(buffer); out: + g_free(buffer); v9fs_string_free(&fullname); return err; }