Re: [Qemu-devel] [PATCH 1/4] 9pfs: drop unused fmt strings in the proxy backend

[Cédric Le Goater]

On 09/15/2016 04:24 PM, Greg Kurz wrote:
> The v9fs_request() function doesn't use its fmt argument: it passes literal
> format strings to proxy_marshal() for all commands.
> 
> This patch simply drops the unused fmt argument and updates all callers
> accordingly.

Reviewed-by: Cédric Le Goater <address@hidden>

> Signed-off-by: Greg Kurz <address@hidden>
> ---
>  hw/9pfs/9p-proxy.c |   67 
> +++++++++++++++++++++++-----------------------------
>  1 file changed, 30 insertions(+), 37 deletions(-)
> 
> diff --git a/hw/9pfs/9p-proxy.c b/hw/9pfs/9p-proxy.c
> index f265501eac1d..52bbf4f1b37c 100644
> --- a/hw/9pfs/9p-proxy.c
> +++ b/hw/9pfs/9p-proxy.c
> @@ -292,12 +292,11 @@ static int v9fs_receive_status(V9fsProxy *proxy,
>  /*
>   * Proxy->header and proxy->request written to socket by QEMU process.
>   * This request read by proxy helper process
>   * returns 0 on success and -errno on error
>   */
> -static int v9fs_request(V9fsProxy *proxy, int type,
> -                        void *response, const char *fmt, ...)
> +static int v9fs_request(V9fsProxy *proxy, int type, void *response, ...)
>  {
>      dev_t rdev;
>      va_list ap;
>      int size = 0;
>      int retval = 0;
> @@ -315,11 +314,11 @@ static int v9fs_request(V9fsProxy *proxy, int type,
>          retval = -EIO;
>          goto err_out;
>      }
>      iovec = &proxy->out_iovec;
>      reply = &proxy->in_iovec;
> -    va_start(ap, fmt);
> +    va_start(ap, response);
>      switch (type) {
>      case T_OPEN:
>          path = va_arg(ap, V9fsString *);
>          flags = va_arg(ap, int);
>          retval = proxy_marshal(iovec, PROXY_HDR_SZ, "sd", path, flags);
> @@ -603,11 +602,11 @@ close_error:
>  }
>  
>  static int proxy_lstat(FsContext *fs_ctx, V9fsPath *fs_path, struct stat 
> *stbuf)
>  {
>      int retval;
> -    retval = v9fs_request(fs_ctx->private, T_LSTAT, stbuf, "s", fs_path);
> +    retval = v9fs_request(fs_ctx->private, T_LSTAT, stbuf, fs_path);
>      if (retval < 0) {
>          errno = -retval;
>          return -1;
>      }
>      return retval;
> @@ -615,12 +614,11 @@ static int proxy_lstat(FsContext *fs_ctx, V9fsPath 
> *fs_path, struct stat *stbuf)
>  
>  static ssize_t proxy_readlink(FsContext *fs_ctx, V9fsPath *fs_path,
>                                char *buf, size_t bufsz)
>  {
>      int retval;
> -    retval = v9fs_request(fs_ctx->private, T_READLINK, buf, "sd",
> -                          fs_path, bufsz);
> +    retval = v9fs_request(fs_ctx->private, T_READLINK, buf, fs_path, bufsz);
>      if (retval < 0) {
>          errno = -retval;
>          return -1;
>      }
>      return strlen(buf);
> @@ -637,11 +635,11 @@ static int proxy_closedir(FsContext *ctx, 
> V9fsFidOpenState *fs)
>  }
>  
>  static int proxy_open(FsContext *ctx, V9fsPath *fs_path,
>                        int flags, V9fsFidOpenState *fs)
>  {
> -    fs->fd = v9fs_request(ctx->private, T_OPEN, NULL, "sd", fs_path, flags);
> +    fs->fd = v9fs_request(ctx->private, T_OPEN, NULL, fs_path, flags);
>      if (fs->fd < 0) {
>          errno = -fs->fd;
>          fs->fd = -1;
>      }
>      return fs->fd;
> @@ -651,11 +649,11 @@ static int proxy_opendir(FsContext *ctx,
>                           V9fsPath *fs_path, V9fsFidOpenState *fs)
>  {
>      int serrno, fd;
>  
>      fs->dir.stream = NULL;
> -    fd = v9fs_request(ctx->private, T_OPEN, NULL, "sd", fs_path, 
> O_DIRECTORY);
> +    fd = v9fs_request(ctx->private, T_OPEN, NULL, fs_path, O_DIRECTORY);
>      if (fd < 0) {
>          errno = -fd;
>          return -1;
>      }
>      fs->dir.stream = fdopendir(fd);
> @@ -733,12 +731,12 @@ static ssize_t proxy_pwritev(FsContext *ctx, 
> V9fsFidOpenState *fs,
>  }
>  
>  static int proxy_chmod(FsContext *fs_ctx, V9fsPath *fs_path, FsCred *credp)
>  {
>      int retval;
> -    retval = v9fs_request(fs_ctx->private, T_CHMOD, NULL, "sd",
> -                          fs_path, credp->fc_mode);
> +    retval = v9fs_request(fs_ctx->private, T_CHMOD, NULL, fs_path,
> +                          credp->fc_mode);
>      if (retval < 0) {
>          errno = -retval;
>      }
>      return retval;
>  }
> @@ -750,12 +748,12 @@ static int proxy_mknod(FsContext *fs_ctx, V9fsPath 
> *dir_path,
>      V9fsString fullname;
>  
>      v9fs_string_init(&fullname);
>      v9fs_string_sprintf(&fullname, "%s/%s", dir_path->data, name);
>  
> -    retval = v9fs_request(fs_ctx->private, T_MKNOD, NULL, "sdqdd",
> -                          &fullname, credp->fc_mode, credp->fc_rdev,
> +    retval = v9fs_request(fs_ctx->private, T_MKNOD, NULL, &fullname,
> +                          credp->fc_mode, credp->fc_rdev,
>                            credp->fc_uid, credp->fc_gid);
>      v9fs_string_free(&fullname);
>      if (retval < 0) {
>          errno = -retval;
>          retval = -1;
> @@ -770,11 +768,11 @@ static int proxy_mkdir(FsContext *fs_ctx, V9fsPath 
> *dir_path,
>      V9fsString fullname;
>  
>      v9fs_string_init(&fullname);
>      v9fs_string_sprintf(&fullname, "%s/%s", dir_path->data, name);
>  
> -    retval = v9fs_request(fs_ctx->private, T_MKDIR, NULL, "sddd", &fullname,
> +    retval = v9fs_request(fs_ctx->private, T_MKDIR, NULL, &fullname,
>                            credp->fc_mode, credp->fc_uid, credp->fc_gid);
>      v9fs_string_free(&fullname);
>      if (retval < 0) {
>          errno = -retval;
>          retval = -1;
> @@ -802,13 +800,12 @@ static int proxy_open2(FsContext *fs_ctx, V9fsPath 
> *dir_path, const char *name,
>      V9fsString fullname;
>  
>      v9fs_string_init(&fullname);
>      v9fs_string_sprintf(&fullname, "%s/%s", dir_path->data, name);
>  
> -    fs->fd = v9fs_request(fs_ctx->private, T_CREATE, NULL, "sdddd",
> -                          &fullname, flags, credp->fc_mode,
> -                          credp->fc_uid, credp->fc_gid);
> +    fs->fd = v9fs_request(fs_ctx->private, T_CREATE, NULL, &fullname, flags,
> +                          credp->fc_mode, credp->fc_uid, credp->fc_gid);
>      v9fs_string_free(&fullname);
>      if (fs->fd < 0) {
>          errno = -fs->fd;
>          fs->fd = -1;
>      }
> @@ -825,12 +822,12 @@ static int proxy_symlink(FsContext *fs_ctx, const char 
> *oldpath,
>      v9fs_string_init(&target);
>  
>      v9fs_string_sprintf(&fullname, "%s/%s", dir_path->data, name);
>      v9fs_string_sprintf(&target, "%s", oldpath);
>  
> -    retval = v9fs_request(fs_ctx->private, T_SYMLINK, NULL, "ssdd",
> -                          &target, &fullname, credp->fc_uid, credp->fc_gid);
> +    retval = v9fs_request(fs_ctx->private, T_SYMLINK, NULL, &target, 
> &fullname,
> +                          credp->fc_uid, credp->fc_gid);
>      v9fs_string_free(&fullname);
>      v9fs_string_free(&target);
>      if (retval < 0) {
>          errno = -retval;
>          retval = -1;
> @@ -845,11 +842,11 @@ static int proxy_link(FsContext *ctx, V9fsPath *oldpath,
>      V9fsString newpath;
>  
>      v9fs_string_init(&newpath);
>      v9fs_string_sprintf(&newpath, "%s/%s", dirpath->data, name);
>  
> -    retval = v9fs_request(ctx->private, T_LINK, NULL, "ss", oldpath, 
> &newpath);
> +    retval = v9fs_request(ctx->private, T_LINK, NULL, oldpath, &newpath);
>      v9fs_string_free(&newpath);
>      if (retval < 0) {
>          errno = -retval;
>          retval = -1;
>      }
> @@ -858,11 +855,11 @@ static int proxy_link(FsContext *ctx, V9fsPath *oldpath,
>  
>  static int proxy_truncate(FsContext *ctx, V9fsPath *fs_path, off_t size)
>  {
>      int retval;
>  
> -    retval = v9fs_request(ctx->private, T_TRUNCATE, NULL, "sq", fs_path, 
> size);
> +    retval = v9fs_request(ctx->private, T_TRUNCATE, NULL, fs_path, size);
>      if (retval < 0) {
>          errno = -retval;
>          return -1;
>      }
>      return 0;
> @@ -877,12 +874,11 @@ static int proxy_rename(FsContext *ctx, const char 
> *oldpath,
>      v9fs_string_init(&oldname);
>      v9fs_string_init(&newname);
>  
>      v9fs_string_sprintf(&oldname, "%s", oldpath);
>      v9fs_string_sprintf(&newname, "%s", newpath);
> -    retval = v9fs_request(ctx->private, T_RENAME, NULL, "ss",
> -                          &oldname, &newname);
> +    retval = v9fs_request(ctx->private, T_RENAME, NULL, &oldname, &newname);
>      v9fs_string_free(&oldname);
>      v9fs_string_free(&newname);
>      if (retval < 0) {
>          errno = -retval;
>      }
> @@ -890,24 +886,23 @@ static int proxy_rename(FsContext *ctx, const char 
> *oldpath,
>  }
>  
>  static int proxy_chown(FsContext *fs_ctx, V9fsPath *fs_path, FsCred *credp)
>  {
>      int retval;
> -    retval = v9fs_request(fs_ctx->private, T_CHOWN, NULL, "sdd",
> -                          fs_path, credp->fc_uid, credp->fc_gid);
> +    retval = v9fs_request(fs_ctx->private, T_CHOWN, NULL, fs_path,
> +                          credp->fc_uid, credp->fc_gid);
>      if (retval < 0) {
>          errno = -retval;
>      }
>      return retval;
>  }
>  
>  static int proxy_utimensat(FsContext *s, V9fsPath *fs_path,
>                             const struct timespec *buf)
>  {
>      int retval;
> -    retval = v9fs_request(s->private, T_UTIME, NULL, "sqqqq",
> -                          fs_path,
> +    retval = v9fs_request(s->private, T_UTIME, NULL, fs_path,
>                            buf[0].tv_sec, buf[0].tv_nsec,
>                            buf[1].tv_sec, buf[1].tv_nsec);
>      if (retval < 0) {
>          errno = -retval;
>      }
> @@ -918,11 +913,11 @@ static int proxy_remove(FsContext *ctx, const char 
> *path)
>  {
>      int retval;
>      V9fsString name;
>      v9fs_string_init(&name);
>      v9fs_string_sprintf(&name, "%s", path);
> -    retval = v9fs_request(ctx->private, T_REMOVE, NULL, "s", &name);
> +    retval = v9fs_request(ctx->private, T_REMOVE, NULL, &name);
>      v9fs_string_free(&name);
>      if (retval < 0) {
>          errno = -retval;
>      }
>      return retval;
> @@ -947,11 +942,11 @@ static int proxy_fsync(FsContext *ctx, int fid_type,
>  }
>  
>  static int proxy_statfs(FsContext *s, V9fsPath *fs_path, struct statfs 
> *stbuf)
>  {
>      int retval;
> -    retval = v9fs_request(s->private, T_STATFS, stbuf, "s", fs_path);
> +    retval = v9fs_request(s->private, T_STATFS, stbuf, fs_path);
>      if (retval < 0) {
>          errno = -retval;
>          return -1;
>      }
>      return retval;
> @@ -963,12 +958,12 @@ static ssize_t proxy_lgetxattr(FsContext *ctx, V9fsPath 
> *fs_path,
>      int retval;
>      V9fsString xname;
>  
>      v9fs_string_init(&xname);
>      v9fs_string_sprintf(&xname, "%s", name);
> -    retval = v9fs_request(ctx->private, T_LGETXATTR, value, "dss", size,
> -                          fs_path, &xname);
> +    retval = v9fs_request(ctx->private, T_LGETXATTR, value, size, fs_path,
> +                          &xname);
>      v9fs_string_free(&xname);
>      if (retval < 0) {
>          errno = -retval;
>      }
>      return retval;
> @@ -976,12 +971,11 @@ static ssize_t proxy_lgetxattr(FsContext *ctx, V9fsPath 
> *fs_path,
>  
>  static ssize_t proxy_llistxattr(FsContext *ctx, V9fsPath *fs_path,
>                                  void *value, size_t size)
>  {
>      int retval;
> -    retval = v9fs_request(ctx->private, T_LLISTXATTR, value, "ds", size,
> -                        fs_path);
> +    retval = v9fs_request(ctx->private, T_LLISTXATTR, value, size, fs_path);
>      if (retval < 0) {
>          errno = -retval;
>      }
>      return retval;
>  }
> @@ -998,12 +992,12 @@ static int proxy_lsetxattr(FsContext *ctx, V9fsPath 
> *fs_path, const char *name,
>      v9fs_string_init(&xvalue);
>      xvalue.size = size;
>      xvalue.data = g_malloc(size);
>      memcpy(xvalue.data, value, size);
>  
> -    retval = v9fs_request(ctx->private, T_LSETXATTR, value, "sssdd",
> -                          fs_path, &xname, &xvalue, size, flags);
> +    retval = v9fs_request(ctx->private, T_LSETXATTR, value, fs_path, &xname,
> +                          &xvalue, size, flags);
>      v9fs_string_free(&xname);
>      v9fs_string_free(&xvalue);
>      if (retval < 0) {
>          errno = -retval;
>      }
> @@ -1016,12 +1010,11 @@ static int proxy_lremovexattr(FsContext *ctx, 
> V9fsPath *fs_path,
>      int retval;
>      V9fsString xname;
>  
>      v9fs_string_init(&xname);
>      v9fs_string_sprintf(&xname, "%s", name);
> -    retval = v9fs_request(ctx->private, T_LREMOVEXATTR, NULL, "ss",
> -                          fs_path, &xname);
> +    retval = v9fs_request(ctx->private, T_LREMOVEXATTR, NULL, fs_path, 
> &xname);
>      v9fs_string_free(&xname);
>      if (retval < 0) {
>          errno = -retval;
>      }
>      return retval;
> @@ -1084,11 +1077,11 @@ static int proxy_ioc_getversion(FsContext *fs_ctx, 
> V9fsPath *path,
>       */
>      if (!S_ISREG(st_mode) && !S_ISDIR(st_mode)) {
>          errno = ENOTTY;
>          return -1;
>      }
> -    err = v9fs_request(fs_ctx->private, T_GETVERSION, st_gen, "s", path);
> +    err = v9fs_request(fs_ctx->private, T_GETVERSION, st_gen, path);
>      if (err < 0) {
>          errno = -err;
>          err = -1;
>      }
>      return err;
> 
>