On Thu, 31 May 2018 21:26:01 -0400
Keno Fischer wrote:
> Both `stbuf` and `local_ioc_getversion` where unused when
> FS_IOC_GETVERSION was not defined, causing a compiler warning.
>
> Reorgnaize the code to avoid this warning.
>
> Signed-off-by: Keno Fischer
> ---
>
> Changes since v1:
> * As request in review, logic is factored into a
>local_ioc_getversion_init function.
>
> hw/9pfs/9p-local.c | 43 +--
> 1 file changed, 25 insertions(+), 18 deletions(-)
>
> diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c
> index 576c8e3..6222891 100644
> --- a/hw/9pfs/9p-local.c
> +++ b/hw/9pfs/9p-local.c
> @@ -1375,10 +1375,10 @@ static int local_unlinkat(FsContext *ctx, V9fsPath
> *dir,
> return ret;
> }
>
> +#ifdef FS_IOC_GETVERSION
> static int local_ioc_getversion(FsContext *ctx, V9fsPath *path,
> mode_t st_mode, uint64_t *st_gen)
> {
> -#ifdef FS_IOC_GETVERSION
> int err;
> V9fsFidOpenState fid_open;
>
> @@ -1397,32 +1397,19 @@ static int local_ioc_getversion(FsContext *ctx,
> V9fsPath *path,
> err = ioctl(fid_open.fd, FS_IOC_GETVERSION, st_gen);
> local_close(ctx, _open);
> return err;
> -#else
> -errno = ENOTTY;
> -return -1;
> -#endif
> }
> +#endif
>
> -static int local_init(FsContext *ctx, Error **errp)
> +static int local_ioc_getversion_init(FsContext *ctx, LocalData *data)
> {
> +#ifdef FS_IOC_GETVERSION
> struct statfs stbuf;
> -LocalData *data = g_malloc(sizeof(*data));
>
> -data->mountfd = open(ctx->fs_root, O_DIRECTORY | O_RDONLY);
> -if (data->mountfd == -1) {
> -error_setg_errno(errp, errno, "failed to open '%s'", ctx->fs_root);
> -goto err;
> -}
> -
> -#ifdef FS_IOC_GETVERSION
> /*
> * use ioc_getversion only if the ioctl is definied
> */
> if (fstatfs(data->mountfd, ) < 0) {
> -close_preserve_errno(data->mountfd);
> -error_setg_errno(errp, errno,
> -"failed to stat file system at '%s'", ctx->fs_root);
> -goto err;
Hmm, I'd prefer to keep the error_setg_errno() with fstatfs(), ie,
add an errp argument to this function.
> +return -1;
> }
> switch (stbuf.f_type) {
> case EXT2_SUPER_MAGIC:
> @@ -1433,6 +1420,26 @@ static int local_init(FsContext *ctx, Error **errp)
> break;
> }
> #endif
> +return 0;
> +}
> +
> +static int local_init(FsContext *ctx, Error **errp)
> +{
> +LocalData *data = g_malloc(sizeof(*data));
> +
> +data->mountfd = open(ctx->fs_root, O_DIRECTORY | O_RDONLY);
> +if (data->mountfd == -1) {
> +error_setg_errno(errp, errno, "failed to open '%s'", ctx->fs_root);
> +goto err;
> +}
> +
> +if (local_ioc_getversion_init(ctx, data) < 0) {
> +close_preserve_errno(data->mountfd);
And this could even be a plain close()
> +error_setg_errno(errp, errno,
> +"failed initialize ioc_getversion for file system at '%s'",
True, but I think "failed to stat file system" is more meaningful,
especially with the errno.
> +ctx->fs_root);
> +goto err;
> +}
>
> if (ctx->export_flags & V9FS_SM_PASSTHROUGH) {
> ctx->xops = passthrough_xattr_ops;
