| Message ID | 0758699faca2ee5526d2b64e0a5fdda7e7845cbc.1527814874.git.keno@juliacomputing.com |
|---|---|
| State | New |
| Headers | show |
| Series | 9p: Add support for Darwin | expand |
On Thu, 31 May 2018 21:26:01 -0400 Keno Fischer <keno@juliacomputing.com> 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 <keno@juliacomputing.com> > --- > > 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, &fid_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, &stbuf) < 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;
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, &fid_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, &stbuf) < 0) { - close_preserve_errno(data->mountfd); - error_setg_errno(errp, errno, - "failed to stat file system at '%s'", ctx->fs_root); - goto err; + 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); + error_setg_errno(errp, errno, + "failed initialize ioc_getversion for file system at '%s'", + ctx->fs_root); + goto err; + } if (ctx->export_flags & V9FS_SM_PASSTHROUGH) { ctx->xops = passthrough_xattr_ops;
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 <keno@juliacomputing.com> --- 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(-)