* [Qemu-devel] [PATCH 0/4] 9pfs: various cleanups
@ 2016-09-15 14:23 Greg Kurz
2016-09-15 14:24 ` [Qemu-devel] [PATCH 1/4] 9pfs: drop unused fmt strings in the proxy backend Greg Kurz
` (3 more replies)
0 siblings, 4 replies; 10+ messages in thread
From: Greg Kurz @ 2016-09-15 14:23 UTC (permalink / raw)
To: qemu-devel; +Cc: Greg Kurz, Aneesh Kumar K.V
These are nearly trivial class code changes. They don't fix any bug, nor change
functionality.
---
Greg Kurz (4):
9pfs: drop unused fmt strings in the proxy backend
9pfs: drop duplicate line in proxy backend
9pfs: drop useless v9fs_string_null() function
9pfs: introduce v9fs_path_sprintf() helper
fsdev/9p-marshal.c | 5 ---
fsdev/9p-marshal.h | 1 -
hw/9pfs/9p-local.c | 7 +----
hw/9pfs/9p-proxy.c | 75 ++++++++++++++++++++++------------------------------
hw/9pfs/9p.c | 27 ++++++++++++++-----
hw/9pfs/9p.h | 1 +
6 files changed, 55 insertions(+), 61 deletions(-)
--
Greg
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Qemu-devel] [PATCH 1/4] 9pfs: drop unused fmt strings in the proxy backend
2016-09-15 14:23 [Qemu-devel] [PATCH 0/4] 9pfs: various cleanups Greg Kurz
@ 2016-09-15 14:24 ` Greg Kurz
2016-09-15 15:10 ` Cédric Le Goater
2016-09-15 14:24 ` [Qemu-devel] [PATCH 2/4] 9pfs: drop duplicate line in " Greg Kurz
` (2 subsequent siblings)
3 siblings, 1 reply; 10+ messages in thread
From: Greg Kurz @ 2016-09-15 14:24 UTC (permalink / raw)
To: qemu-devel; +Cc: Greg Kurz, Aneesh Kumar K.V
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.
Signed-off-by: Greg Kurz <groug@kaod.org>
---
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;
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [Qemu-devel] [PATCH 2/4] 9pfs: drop duplicate line in proxy backend
2016-09-15 14:23 [Qemu-devel] [PATCH 0/4] 9pfs: various cleanups Greg Kurz
2016-09-15 14:24 ` [Qemu-devel] [PATCH 1/4] 9pfs: drop unused fmt strings in the proxy backend Greg Kurz
@ 2016-09-15 14:24 ` Greg Kurz
2016-09-15 15:05 ` Cédric Le Goater
2016-09-15 14:24 ` [Qemu-devel] [PATCH 3/4] 9pfs: drop useless v9fs_string_null() function Greg Kurz
2016-09-15 14:24 ` [Qemu-devel] [PATCH 4/4] 9pfs: introduce v9fs_path_sprintf() helper Greg Kurz
3 siblings, 1 reply; 10+ messages in thread
From: Greg Kurz @ 2016-09-15 14:24 UTC (permalink / raw)
To: qemu-devel; +Cc: Greg Kurz, Aneesh Kumar K.V
This double free did not cause harm because v9fs_string_free() sets
str->data to NULL and g_free(NULL) is valid.
Signed-off-by: Greg Kurz <groug@kaod.org>
---
hw/9pfs/9p-proxy.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/hw/9pfs/9p-proxy.c b/hw/9pfs/9p-proxy.c
index 52bbf4f1b37c..d091564b6fd2 100644
--- a/hw/9pfs/9p-proxy.c
+++ b/hw/9pfs/9p-proxy.c
@@ -775,11 +775,10 @@ static int proxy_mkdir(FsContext *fs_ctx, V9fsPath *dir_path,
v9fs_string_free(&fullname);
if (retval < 0) {
errno = -retval;
retval = -1;
}
- v9fs_string_free(&fullname);
return retval;
}
static int proxy_fstat(FsContext *fs_ctx, int fid_type,
V9fsFidOpenState *fs, struct stat *stbuf)
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [Qemu-devel] [PATCH 3/4] 9pfs: drop useless v9fs_string_null() function
2016-09-15 14:23 [Qemu-devel] [PATCH 0/4] 9pfs: various cleanups Greg Kurz
2016-09-15 14:24 ` [Qemu-devel] [PATCH 1/4] 9pfs: drop unused fmt strings in the proxy backend Greg Kurz
2016-09-15 14:24 ` [Qemu-devel] [PATCH 2/4] 9pfs: drop duplicate line in " Greg Kurz
@ 2016-09-15 14:24 ` Greg Kurz
2016-09-15 15:06 ` Cédric Le Goater
2016-09-15 14:24 ` [Qemu-devel] [PATCH 4/4] 9pfs: introduce v9fs_path_sprintf() helper Greg Kurz
3 siblings, 1 reply; 10+ messages in thread
From: Greg Kurz @ 2016-09-15 14:24 UTC (permalink / raw)
To: qemu-devel; +Cc: Greg Kurz, Aneesh Kumar K.V
The v9fs_string_null() function just calls v9fs_string_free(). Also it
only has 4 users, whereas v9fs_string_free() has 87.
This patch converts users to call directly v9fs_string_free() and drops
the useless function.
Signed-off-by: Greg Kurz <groug@kaod.org>
---
fsdev/9p-marshal.c | 5 -----
fsdev/9p-marshal.h | 1 -
hw/9pfs/9p.c | 8 ++++----
3 files changed, 4 insertions(+), 10 deletions(-)
diff --git a/fsdev/9p-marshal.c b/fsdev/9p-marshal.c
index 238dbf21b1d5..a01bba6908a8 100644
--- a/fsdev/9p-marshal.c
+++ b/fsdev/9p-marshal.c
@@ -23,15 +23,10 @@ void v9fs_string_free(V9fsString *str)
g_free(str->data);
str->data = NULL;
str->size = 0;
}
-void v9fs_string_null(V9fsString *str)
-{
- v9fs_string_free(str);
-}
-
void GCC_FMT_ATTR(2, 3)
v9fs_string_sprintf(V9fsString *str, const char *fmt, ...)
{
va_list ap;
diff --git a/fsdev/9p-marshal.h b/fsdev/9p-marshal.h
index 140db6d99f9c..77f7fef326ee 100644
--- a/fsdev/9p-marshal.h
+++ b/fsdev/9p-marshal.h
@@ -75,10 +75,9 @@ static inline void v9fs_string_init(V9fsString *str)
{
str->data = NULL;
str->size = 0;
}
extern void v9fs_string_free(V9fsString *str);
-extern void v9fs_string_null(V9fsString *str);
extern void v9fs_string_sprintf(V9fsString *str, const char *fmt, ...);
extern void v9fs_string_copy(V9fsString *lhs, V9fsString *rhs);
#endif
diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index dfe293d11d1c..d8f48ca76c47 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -808,19 +808,19 @@ static int stat_to_v9stat(V9fsPDU *pdu, V9fsPath *name,
v9stat->mode = stat_to_v9mode(stbuf);
v9stat->atime = stbuf->st_atime;
v9stat->mtime = stbuf->st_mtime;
v9stat->length = stbuf->st_size;
- v9fs_string_null(&v9stat->uid);
- v9fs_string_null(&v9stat->gid);
- v9fs_string_null(&v9stat->muid);
+ v9fs_string_free(&v9stat->uid);
+ v9fs_string_free(&v9stat->gid);
+ v9fs_string_free(&v9stat->muid);
v9stat->n_uid = stbuf->st_uid;
v9stat->n_gid = stbuf->st_gid;
v9stat->n_muid = 0;
- v9fs_string_null(&v9stat->extension);
+ v9fs_string_free(&v9stat->extension);
if (v9stat->mode & P9_STAT_MODE_SYMLINK) {
err = v9fs_co_readlink(pdu, name, &v9stat->extension);
if (err < 0) {
return err;
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [Qemu-devel] [PATCH 4/4] 9pfs: introduce v9fs_path_sprintf() helper
2016-09-15 14:23 [Qemu-devel] [PATCH 0/4] 9pfs: various cleanups Greg Kurz
` (2 preceding siblings ...)
2016-09-15 14:24 ` [Qemu-devel] [PATCH 3/4] 9pfs: drop useless v9fs_string_null() function Greg Kurz
@ 2016-09-15 14:24 ` Greg Kurz
2016-09-15 15:09 ` Cédric Le Goater
3 siblings, 1 reply; 10+ messages in thread
From: Greg Kurz @ 2016-09-15 14:24 UTC (permalink / raw)
To: qemu-devel; +Cc: Greg Kurz, Aneesh Kumar K.V
This helper is similar to v9fs_string_sprintf(), but it includes the
terminating NUL character in the size field.
This is to avoid doing v9fs_string_sprintf((V9fsString *) &path) and
then bumping the size.
Affected users are changed to use this new helper.
Signed-off-by: Greg Kurz <groug@kaod.org>
---
hw/9pfs/9p-local.c | 7 ++-----
hw/9pfs/9p-proxy.c | 7 ++-----
hw/9pfs/9p.c | 19 ++++++++++++++++---
hw/9pfs/9p.h | 1 +
4 files changed, 21 insertions(+), 13 deletions(-)
diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c
index 3f271fcbd2c5..845675e7a1bb 100644
--- a/hw/9pfs/9p-local.c
+++ b/hw/9pfs/9p-local.c
@@ -1058,17 +1058,14 @@ static int local_lremovexattr(FsContext *ctx, V9fsPath *fs_path,
static int local_name_to_path(FsContext *ctx, V9fsPath *dir_path,
const char *name, V9fsPath *target)
{
if (dir_path) {
- v9fs_string_sprintf((V9fsString *)target, "%s/%s",
- dir_path->data, name);
+ v9fs_path_sprintf(target, "%s/%s", dir_path->data, name);
} else {
- v9fs_string_sprintf((V9fsString *)target, "%s", name);
+ v9fs_path_sprintf(target, "%s", name);
}
- /* Bump the size for including terminating NULL */
- target->size++;
return 0;
}
static int local_renameat(FsContext *ctx, V9fsPath *olddir,
const char *old_name, V9fsPath *newdir,
diff --git a/hw/9pfs/9p-proxy.c b/hw/9pfs/9p-proxy.c
index d091564b6fd2..f2417b7fd73d 100644
--- a/hw/9pfs/9p-proxy.c
+++ b/hw/9pfs/9p-proxy.c
@@ -1021,17 +1021,14 @@ static int proxy_lremovexattr(FsContext *ctx, V9fsPath *fs_path,
static int proxy_name_to_path(FsContext *ctx, V9fsPath *dir_path,
const char *name, V9fsPath *target)
{
if (dir_path) {
- v9fs_string_sprintf((V9fsString *)target, "%s/%s",
- dir_path->data, name);
+ v9fs_path_sprintf(target, "%s/%s", dir_path->data, name);
} else {
- v9fs_string_sprintf((V9fsString *)target, "%s", name);
+ v9fs_path_sprintf(target, "%s", name);
}
- /* Bump the size for including terminating NULL */
- target->size++;
return 0;
}
static int proxy_renameat(FsContext *ctx, V9fsPath *olddir,
const char *old_name, V9fsPath *newdir,
diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index d8f48ca76c47..639f93930285 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -10,10 +10,11 @@
* the COPYING file in the top-level directory.
*
*/
#include "qemu/osdep.h"
+#include <glib/gprintf.h>
#include "hw/virtio/virtio.h"
#include "qapi/error.h"
#include "qemu/error-report.h"
#include "qemu/iov.h"
#include "qemu/sockets.h"
@@ -177,10 +178,24 @@ void v9fs_path_free(V9fsPath *path)
g_free(path->data);
path->data = NULL;
path->size = 0;
}
+
+void GCC_FMT_ATTR(2, 3)
+v9fs_path_sprintf(V9fsPath *path, const char *fmt, ...)
+{
+ va_list ap;
+
+ v9fs_path_free(path);
+
+ va_start(ap, fmt);
+ /* Bump the size for including terminating NULL */
+ path->size = g_vasprintf(&path->data, fmt, ap) + 1;
+ va_end(ap);
+}
+
void v9fs_path_copy(V9fsPath *lhs, V9fsPath *rhs)
{
v9fs_path_free(lhs);
lhs->data = g_malloc(rhs->size);
memcpy(lhs->data, rhs->data, rhs->size);
@@ -915,14 +930,12 @@ static void print_sg(struct iovec *sg, int cnt)
static void v9fs_fix_path(V9fsPath *dst, V9fsPath *src, int len)
{
V9fsPath str;
v9fs_path_init(&str);
v9fs_path_copy(&str, dst);
- v9fs_string_sprintf((V9fsString *)dst, "%s%s", src->data, str.data+len);
+ v9fs_path_sprintf(dst, "%s%s", src->data, str.data + len);
v9fs_path_free(&str);
- /* +1 to include terminating NULL */
- dst->size++;
}
static inline bool is_ro_export(FsContext *ctx)
{
return ctx->export_flags & V9FS_RDONLY;
diff --git a/hw/9pfs/9p.h b/hw/9pfs/9p.h
index a38603398ef5..d539d2ebe9c0 100644
--- a/hw/9pfs/9p.h
+++ b/hw/9pfs/9p.h
@@ -325,10 +325,11 @@ static inline uint8_t v9fs_request_cancelled(V9fsPDU *pdu)
}
extern void v9fs_reclaim_fd(V9fsPDU *pdu);
extern void v9fs_path_init(V9fsPath *path);
extern void v9fs_path_free(V9fsPath *path);
+extern void v9fs_path_sprintf(V9fsPath *path, const char *fmt, ...);
extern void v9fs_path_copy(V9fsPath *lhs, V9fsPath *rhs);
extern int v9fs_name_to_path(V9fsState *s, V9fsPath *dirpath,
const char *name, V9fsPath *path);
extern int v9fs_device_realize_common(V9fsState *s, Error **errp);
extern void v9fs_device_unrealize_common(V9fsState *s, Error **errp);
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH 2/4] 9pfs: drop duplicate line in proxy backend
2016-09-15 14:24 ` [Qemu-devel] [PATCH 2/4] 9pfs: drop duplicate line in " Greg Kurz
@ 2016-09-15 15:05 ` Cédric Le Goater
0 siblings, 0 replies; 10+ messages in thread
From: Cédric Le Goater @ 2016-09-15 15:05 UTC (permalink / raw)
To: Greg Kurz, qemu-devel; +Cc: Aneesh Kumar K.V
On 09/15/2016 04:24 PM, Greg Kurz wrote:
> This double free did not cause harm because v9fs_string_free() sets
> str->data to NULL and g_free(NULL) is valid.
>
> Signed-off-by: Greg Kurz <groug@kaod.org>
Reviewed-by: Cédric Le Goater <clg@kaod.org>
> ---
> hw/9pfs/9p-proxy.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/hw/9pfs/9p-proxy.c b/hw/9pfs/9p-proxy.c
> index 52bbf4f1b37c..d091564b6fd2 100644
> --- a/hw/9pfs/9p-proxy.c
> +++ b/hw/9pfs/9p-proxy.c
> @@ -775,11 +775,10 @@ static int proxy_mkdir(FsContext *fs_ctx, V9fsPath *dir_path,
> v9fs_string_free(&fullname);
> if (retval < 0) {
> errno = -retval;
> retval = -1;
> }
> - v9fs_string_free(&fullname);
> return retval;
> }
>
> static int proxy_fstat(FsContext *fs_ctx, int fid_type,
> V9fsFidOpenState *fs, struct stat *stbuf)
>
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH 3/4] 9pfs: drop useless v9fs_string_null() function
2016-09-15 14:24 ` [Qemu-devel] [PATCH 3/4] 9pfs: drop useless v9fs_string_null() function Greg Kurz
@ 2016-09-15 15:06 ` Cédric Le Goater
0 siblings, 0 replies; 10+ messages in thread
From: Cédric Le Goater @ 2016-09-15 15:06 UTC (permalink / raw)
To: Greg Kurz, qemu-devel; +Cc: Aneesh Kumar K.V
On 09/15/2016 04:24 PM, Greg Kurz wrote:
> The v9fs_string_null() function just calls v9fs_string_free(). Also it
> only has 4 users, whereas v9fs_string_free() has 87.
>
> This patch converts users to call directly v9fs_string_free() and drops
> the useless function.
Reviewed-by: Cédric Le Goater <clg@kaod.org>
> Signed-off-by: Greg Kurz <groug@kaod.org>
> ---
> fsdev/9p-marshal.c | 5 -----
> fsdev/9p-marshal.h | 1 -
> hw/9pfs/9p.c | 8 ++++----
> 3 files changed, 4 insertions(+), 10 deletions(-)
>
> diff --git a/fsdev/9p-marshal.c b/fsdev/9p-marshal.c
> index 238dbf21b1d5..a01bba6908a8 100644
> --- a/fsdev/9p-marshal.c
> +++ b/fsdev/9p-marshal.c
> @@ -23,15 +23,10 @@ void v9fs_string_free(V9fsString *str)
> g_free(str->data);
> str->data = NULL;
> str->size = 0;
> }
>
> -void v9fs_string_null(V9fsString *str)
> -{
> - v9fs_string_free(str);
> -}
> -
> void GCC_FMT_ATTR(2, 3)
> v9fs_string_sprintf(V9fsString *str, const char *fmt, ...)
> {
> va_list ap;
>
> diff --git a/fsdev/9p-marshal.h b/fsdev/9p-marshal.h
> index 140db6d99f9c..77f7fef326ee 100644
> --- a/fsdev/9p-marshal.h
> +++ b/fsdev/9p-marshal.h
> @@ -75,10 +75,9 @@ static inline void v9fs_string_init(V9fsString *str)
> {
> str->data = NULL;
> str->size = 0;
> }
> extern void v9fs_string_free(V9fsString *str);
> -extern void v9fs_string_null(V9fsString *str);
> extern void v9fs_string_sprintf(V9fsString *str, const char *fmt, ...);
> extern void v9fs_string_copy(V9fsString *lhs, V9fsString *rhs);
>
> #endif
> diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
> index dfe293d11d1c..d8f48ca76c47 100644
> --- a/hw/9pfs/9p.c
> +++ b/hw/9pfs/9p.c
> @@ -808,19 +808,19 @@ static int stat_to_v9stat(V9fsPDU *pdu, V9fsPath *name,
> v9stat->mode = stat_to_v9mode(stbuf);
> v9stat->atime = stbuf->st_atime;
> v9stat->mtime = stbuf->st_mtime;
> v9stat->length = stbuf->st_size;
>
> - v9fs_string_null(&v9stat->uid);
> - v9fs_string_null(&v9stat->gid);
> - v9fs_string_null(&v9stat->muid);
> + v9fs_string_free(&v9stat->uid);
> + v9fs_string_free(&v9stat->gid);
> + v9fs_string_free(&v9stat->muid);
>
> v9stat->n_uid = stbuf->st_uid;
> v9stat->n_gid = stbuf->st_gid;
> v9stat->n_muid = 0;
>
> - v9fs_string_null(&v9stat->extension);
> + v9fs_string_free(&v9stat->extension);
>
> if (v9stat->mode & P9_STAT_MODE_SYMLINK) {
> err = v9fs_co_readlink(pdu, name, &v9stat->extension);
> if (err < 0) {
> return err;
>
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH 4/4] 9pfs: introduce v9fs_path_sprintf() helper
2016-09-15 14:24 ` [Qemu-devel] [PATCH 4/4] 9pfs: introduce v9fs_path_sprintf() helper Greg Kurz
@ 2016-09-15 15:09 ` Cédric Le Goater
2016-09-15 15:24 ` Greg Kurz
0 siblings, 1 reply; 10+ messages in thread
From: Cédric Le Goater @ 2016-09-15 15:09 UTC (permalink / raw)
To: Greg Kurz, qemu-devel; +Cc: Aneesh Kumar K.V
On 09/15/2016 04:24 PM, Greg Kurz wrote:
> This helper is similar to v9fs_string_sprintf(), but it includes the
> terminating NUL character in the size field.
NULL
> This is to avoid doing v9fs_string_sprintf((V9fsString *) &path) and
> then bumping the size.
>
> Affected users are changed to use this new helper.
Reviewed-by: Cédric Le Goater <clg@kaod.org>
> Signed-off-by: Greg Kurz <groug@kaod.org>
> ---
> hw/9pfs/9p-local.c | 7 ++-----
> hw/9pfs/9p-proxy.c | 7 ++-----
> hw/9pfs/9p.c | 19 ++++++++++++++++---
> hw/9pfs/9p.h | 1 +
> 4 files changed, 21 insertions(+), 13 deletions(-)
>
> diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c
> index 3f271fcbd2c5..845675e7a1bb 100644
> --- a/hw/9pfs/9p-local.c
> +++ b/hw/9pfs/9p-local.c
> @@ -1058,17 +1058,14 @@ static int local_lremovexattr(FsContext *ctx, V9fsPath *fs_path,
>
> static int local_name_to_path(FsContext *ctx, V9fsPath *dir_path,
> const char *name, V9fsPath *target)
> {
> if (dir_path) {
> - v9fs_string_sprintf((V9fsString *)target, "%s/%s",
> - dir_path->data, name);
> + v9fs_path_sprintf(target, "%s/%s", dir_path->data, name);
> } else {
> - v9fs_string_sprintf((V9fsString *)target, "%s", name);
> + v9fs_path_sprintf(target, "%s", name);
> }
> - /* Bump the size for including terminating NULL */
> - target->size++;
> return 0;
> }
>
> static int local_renameat(FsContext *ctx, V9fsPath *olddir,
> const char *old_name, V9fsPath *newdir,
> diff --git a/hw/9pfs/9p-proxy.c b/hw/9pfs/9p-proxy.c
> index d091564b6fd2..f2417b7fd73d 100644
> --- a/hw/9pfs/9p-proxy.c
> +++ b/hw/9pfs/9p-proxy.c
> @@ -1021,17 +1021,14 @@ static int proxy_lremovexattr(FsContext *ctx, V9fsPath *fs_path,
>
> static int proxy_name_to_path(FsContext *ctx, V9fsPath *dir_path,
> const char *name, V9fsPath *target)
> {
> if (dir_path) {
> - v9fs_string_sprintf((V9fsString *)target, "%s/%s",
> - dir_path->data, name);
> + v9fs_path_sprintf(target, "%s/%s", dir_path->data, name);
> } else {
> - v9fs_string_sprintf((V9fsString *)target, "%s", name);
> + v9fs_path_sprintf(target, "%s", name);
> }
> - /* Bump the size for including terminating NULL */
> - target->size++;
> return 0;
> }
>
> static int proxy_renameat(FsContext *ctx, V9fsPath *olddir,
> const char *old_name, V9fsPath *newdir,
> diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
> index d8f48ca76c47..639f93930285 100644
> --- a/hw/9pfs/9p.c
> +++ b/hw/9pfs/9p.c
> @@ -10,10 +10,11 @@
> * the COPYING file in the top-level directory.
> *
> */
>
> #include "qemu/osdep.h"
> +#include <glib/gprintf.h>
> #include "hw/virtio/virtio.h"
> #include "qapi/error.h"
> #include "qemu/error-report.h"
> #include "qemu/iov.h"
> #include "qemu/sockets.h"
> @@ -177,10 +178,24 @@ void v9fs_path_free(V9fsPath *path)
> g_free(path->data);
> path->data = NULL;
> path->size = 0;
> }
>
> +
> +void GCC_FMT_ATTR(2, 3)
> +v9fs_path_sprintf(V9fsPath *path, const char *fmt, ...)
> +{
> + va_list ap;
> +
> + v9fs_path_free(path);
> +
> + va_start(ap, fmt);
> + /* Bump the size for including terminating NULL */
> + path->size = g_vasprintf(&path->data, fmt, ap) + 1;
> + va_end(ap);
> +}
> +
> void v9fs_path_copy(V9fsPath *lhs, V9fsPath *rhs)
> {
> v9fs_path_free(lhs);
> lhs->data = g_malloc(rhs->size);
> memcpy(lhs->data, rhs->data, rhs->size);
> @@ -915,14 +930,12 @@ static void print_sg(struct iovec *sg, int cnt)
> static void v9fs_fix_path(V9fsPath *dst, V9fsPath *src, int len)
> {
> V9fsPath str;
> v9fs_path_init(&str);
> v9fs_path_copy(&str, dst);
> - v9fs_string_sprintf((V9fsString *)dst, "%s%s", src->data, str.data+len);
> + v9fs_path_sprintf(dst, "%s%s", src->data, str.data + len);
> v9fs_path_free(&str);
> - /* +1 to include terminating NULL */
> - dst->size++;
> }
>
> static inline bool is_ro_export(FsContext *ctx)
> {
> return ctx->export_flags & V9FS_RDONLY;
> diff --git a/hw/9pfs/9p.h b/hw/9pfs/9p.h
> index a38603398ef5..d539d2ebe9c0 100644
> --- a/hw/9pfs/9p.h
> +++ b/hw/9pfs/9p.h
> @@ -325,10 +325,11 @@ static inline uint8_t v9fs_request_cancelled(V9fsPDU *pdu)
> }
>
> extern void v9fs_reclaim_fd(V9fsPDU *pdu);
> extern void v9fs_path_init(V9fsPath *path);
> extern void v9fs_path_free(V9fsPath *path);
> +extern void v9fs_path_sprintf(V9fsPath *path, const char *fmt, ...);
> extern void v9fs_path_copy(V9fsPath *lhs, V9fsPath *rhs);
> extern int v9fs_name_to_path(V9fsState *s, V9fsPath *dirpath,
> const char *name, V9fsPath *path);
> extern int v9fs_device_realize_common(V9fsState *s, Error **errp);
> extern void v9fs_device_unrealize_common(V9fsState *s, Error **errp);
>
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH 1/4] 9pfs: drop unused fmt strings in the proxy backend
2016-09-15 14:24 ` [Qemu-devel] [PATCH 1/4] 9pfs: drop unused fmt strings in the proxy backend Greg Kurz
@ 2016-09-15 15:10 ` Cédric Le Goater
0 siblings, 0 replies; 10+ messages in thread
From: Cédric Le Goater @ 2016-09-15 15:10 UTC (permalink / raw)
To: Greg Kurz, qemu-devel; +Cc: Aneesh Kumar K.V
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 <clg@kaod.org>
> Signed-off-by: Greg Kurz <groug@kaod.org>
> ---
> 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;
>
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH 4/4] 9pfs: introduce v9fs_path_sprintf() helper
2016-09-15 15:09 ` Cédric Le Goater
@ 2016-09-15 15:24 ` Greg Kurz
0 siblings, 0 replies; 10+ messages in thread
From: Greg Kurz @ 2016-09-15 15:24 UTC (permalink / raw)
To: Cédric Le Goater; +Cc: qemu-devel, Aneesh Kumar K.V
On Thu, 15 Sep 2016 17:09:08 +0200
Cédric Le Goater <clg@kaod.org> wrote:
> On 09/15/2016 04:24 PM, Greg Kurz wrote:
> > This helper is similar to v9fs_string_sprintf(), but it includes the
> > terminating NUL character in the size field.
>
> NULL
>
The null character is often abbreviated NUL.
https://en.wikipedia.org/wiki/Null_character
> > This is to avoid doing v9fs_string_sprintf((V9fsString *) &path) and
> > then bumping the size.
> >
> > Affected users are changed to use this new helper.
>
>
>
> Reviewed-by: Cédric Le Goater <clg@kaod.org>
>
>
> > Signed-off-by: Greg Kurz <groug@kaod.org>
> > ---
> > hw/9pfs/9p-local.c | 7 ++-----
> > hw/9pfs/9p-proxy.c | 7 ++-----
> > hw/9pfs/9p.c | 19 ++++++++++++++++---
> > hw/9pfs/9p.h | 1 +
> > 4 files changed, 21 insertions(+), 13 deletions(-)
> >
> > diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c
> > index 3f271fcbd2c5..845675e7a1bb 100644
> > --- a/hw/9pfs/9p-local.c
> > +++ b/hw/9pfs/9p-local.c
> > @@ -1058,17 +1058,14 @@ static int local_lremovexattr(FsContext *ctx, V9fsPath *fs_path,
> >
> > static int local_name_to_path(FsContext *ctx, V9fsPath *dir_path,
> > const char *name, V9fsPath *target)
> > {
> > if (dir_path) {
> > - v9fs_string_sprintf((V9fsString *)target, "%s/%s",
> > - dir_path->data, name);
> > + v9fs_path_sprintf(target, "%s/%s", dir_path->data, name);
> > } else {
> > - v9fs_string_sprintf((V9fsString *)target, "%s", name);
> > + v9fs_path_sprintf(target, "%s", name);
> > }
> > - /* Bump the size for including terminating NULL */
> > - target->size++;
> > return 0;
> > }
> >
> > static int local_renameat(FsContext *ctx, V9fsPath *olddir,
> > const char *old_name, V9fsPath *newdir,
> > diff --git a/hw/9pfs/9p-proxy.c b/hw/9pfs/9p-proxy.c
> > index d091564b6fd2..f2417b7fd73d 100644
> > --- a/hw/9pfs/9p-proxy.c
> > +++ b/hw/9pfs/9p-proxy.c
> > @@ -1021,17 +1021,14 @@ static int proxy_lremovexattr(FsContext *ctx, V9fsPath *fs_path,
> >
> > static int proxy_name_to_path(FsContext *ctx, V9fsPath *dir_path,
> > const char *name, V9fsPath *target)
> > {
> > if (dir_path) {
> > - v9fs_string_sprintf((V9fsString *)target, "%s/%s",
> > - dir_path->data, name);
> > + v9fs_path_sprintf(target, "%s/%s", dir_path->data, name);
> > } else {
> > - v9fs_string_sprintf((V9fsString *)target, "%s", name);
> > + v9fs_path_sprintf(target, "%s", name);
> > }
> > - /* Bump the size for including terminating NULL */
> > - target->size++;
> > return 0;
> > }
> >
> > static int proxy_renameat(FsContext *ctx, V9fsPath *olddir,
> > const char *old_name, V9fsPath *newdir,
> > diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
> > index d8f48ca76c47..639f93930285 100644
> > --- a/hw/9pfs/9p.c
> > +++ b/hw/9pfs/9p.c
> > @@ -10,10 +10,11 @@
> > * the COPYING file in the top-level directory.
> > *
> > */
> >
> > #include "qemu/osdep.h"
> > +#include <glib/gprintf.h>
> > #include "hw/virtio/virtio.h"
> > #include "qapi/error.h"
> > #include "qemu/error-report.h"
> > #include "qemu/iov.h"
> > #include "qemu/sockets.h"
> > @@ -177,10 +178,24 @@ void v9fs_path_free(V9fsPath *path)
> > g_free(path->data);
> > path->data = NULL;
> > path->size = 0;
> > }
> >
> > +
> > +void GCC_FMT_ATTR(2, 3)
> > +v9fs_path_sprintf(V9fsPath *path, const char *fmt, ...)
> > +{
> > + va_list ap;
> > +
> > + v9fs_path_free(path);
> > +
> > + va_start(ap, fmt);
> > + /* Bump the size for including terminating NULL */
> > + path->size = g_vasprintf(&path->data, fmt, ap) + 1;
> > + va_end(ap);
> > +}
> > +
> > void v9fs_path_copy(V9fsPath *lhs, V9fsPath *rhs)
> > {
> > v9fs_path_free(lhs);
> > lhs->data = g_malloc(rhs->size);
> > memcpy(lhs->data, rhs->data, rhs->size);
> > @@ -915,14 +930,12 @@ static void print_sg(struct iovec *sg, int cnt)
> > static void v9fs_fix_path(V9fsPath *dst, V9fsPath *src, int len)
> > {
> > V9fsPath str;
> > v9fs_path_init(&str);
> > v9fs_path_copy(&str, dst);
> > - v9fs_string_sprintf((V9fsString *)dst, "%s%s", src->data, str.data+len);
> > + v9fs_path_sprintf(dst, "%s%s", src->data, str.data + len);
> > v9fs_path_free(&str);
> > - /* +1 to include terminating NULL */
> > - dst->size++;
> > }
> >
> > static inline bool is_ro_export(FsContext *ctx)
> > {
> > return ctx->export_flags & V9FS_RDONLY;
> > diff --git a/hw/9pfs/9p.h b/hw/9pfs/9p.h
> > index a38603398ef5..d539d2ebe9c0 100644
> > --- a/hw/9pfs/9p.h
> > +++ b/hw/9pfs/9p.h
> > @@ -325,10 +325,11 @@ static inline uint8_t v9fs_request_cancelled(V9fsPDU *pdu)
> > }
> >
> > extern void v9fs_reclaim_fd(V9fsPDU *pdu);
> > extern void v9fs_path_init(V9fsPath *path);
> > extern void v9fs_path_free(V9fsPath *path);
> > +extern void v9fs_path_sprintf(V9fsPath *path, const char *fmt, ...);
> > extern void v9fs_path_copy(V9fsPath *lhs, V9fsPath *rhs);
> > extern int v9fs_name_to_path(V9fsState *s, V9fsPath *dirpath,
> > const char *name, V9fsPath *path);
> > extern int v9fs_device_realize_common(V9fsState *s, Error **errp);
> > extern void v9fs_device_unrealize_common(V9fsState *s, Error **errp);
> >
> >
>
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2016-09-15 15:25 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-09-15 14:23 [Qemu-devel] [PATCH 0/4] 9pfs: various cleanups Greg Kurz
2016-09-15 14:24 ` [Qemu-devel] [PATCH 1/4] 9pfs: drop unused fmt strings in the proxy backend Greg Kurz
2016-09-15 15:10 ` Cédric Le Goater
2016-09-15 14:24 ` [Qemu-devel] [PATCH 2/4] 9pfs: drop duplicate line in " Greg Kurz
2016-09-15 15:05 ` Cédric Le Goater
2016-09-15 14:24 ` [Qemu-devel] [PATCH 3/4] 9pfs: drop useless v9fs_string_null() function Greg Kurz
2016-09-15 15:06 ` Cédric Le Goater
2016-09-15 14:24 ` [Qemu-devel] [PATCH 4/4] 9pfs: introduce v9fs_path_sprintf() helper Greg Kurz
2016-09-15 15:09 ` Cédric Le Goater
2016-09-15 15:24 ` Greg Kurz
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).