From: Venkateswararao Jujjuri <jvrao@linux.vnet.ibm.com>
To: qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCH] Clean up virtio-9p error handling code
Date: Fri, 13 May 2011 16:59:49 -0700 [thread overview]
Message-ID: <4DCDC5F5.9090508@linux.vnet.ibm.com> (raw)
In-Reply-To: <BANLkTik+PQvicw3Shx7TGQymT0YRWHdaJw@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 46742 bytes --]
On 05/09/2011 02:21 AM, Sassan Panahinejad wrote:
> On 8 May 2011 19:34, Venkateswararao Jujjuri <jvrao@linux.vnet.ibm.com
> <mailto:jvrao@linux.vnet.ibm.com>> wrote:
>
> On 05/08/2011 05:36 AM, Sassan Panahinejad wrote:
>
> In a lot of cases, the handling of errors was quite ugly.
> This patch moves reading of errno to immediately after the
> system calls and passes it up through the system more cleanly.
> Also, in the case of the xattr functions (and possibly
> others), completely the wrong error was being returned.
>
signed-off-by ??
Also make sure that the patch passes . This patch is done on windows
format. It doesn't apply.
scripts/checkpatch.pl <patch>
> Given the nature of this patch this may need to be tested properly.
> I will pull this into our test infrastructure and let it brew for
> a week-or-so.
> In the mean while can you please describe your validation/testing ?
> May be best place to start with is
> http://www.tuxera.com/community/posix-test-suite/
>
>
>
> I have now run the tests you suggested and it passes them all.
> I've also done some informal checks.
>
> Thanks for the advice
> Sassan
>
>
>
> Thanks,
> JV
>
>
> ---
> fsdev/file-op-9p.h | 4 +-
> hw/9pfs/virtio-9p-local.c | 121
> ++++++++++++++++++++----------------
> hw/9pfs/virtio-9p-xattr.c | 21 ++++++-
> hw/9pfs/virtio-9p.c | 150
> +++++++++++---------------------------------
> 4 files changed, 125 insertions(+), 171 deletions(-)
>
> diff --git a/fsdev/file-op-9p.h b/fsdev/file-op-9p.h
> index 126e60e..81a822a 100644
> --- a/fsdev/file-op-9p.h
> +++ b/fsdev/file-op-9p.h
> @@ -73,12 +73,12 @@ typedef struct FileOperations
> int (*setuid)(FsContext *, uid_t);
> int (*close)(FsContext *, int);
> int (*closedir)(FsContext *, DIR *);
> - DIR *(*opendir)(FsContext *, const char *);
> + int (*opendir)(FsContext *, const char *, DIR **);
> int (*open)(FsContext *, const char *, int);
> int (*open2)(FsContext *, const char *, int, FsCred *);
> void (*rewinddir)(FsContext *, DIR *);
> off_t (*telldir)(FsContext *, DIR *);
> - struct dirent *(*readdir)(FsContext *, DIR *);
> + int (*readdir)(FsContext *, DIR *, struct dirent **);
> void (*seekdir)(FsContext *, DIR *, off_t);
> ssize_t (*preadv)(FsContext *, int, const struct iovec *,
> int, off_t);
> ssize_t (*pwritev)(FsContext *, int, const struct iovec
> *, int, off_t);
> diff --git a/hw/9pfs/virtio-9p-local.c b/hw/9pfs/virtio-9p-local.c
> index 0a015de..afaff78 100644
> --- a/hw/9pfs/virtio-9p-local.c
> +++ b/hw/9pfs/virtio-9p-local.c
> @@ -26,7 +26,7 @@ static int local_lstat(FsContext *fs_ctx,
> const char *path, struct stat *stbuf)
> int err;
> err = lstat(rpath(fs_ctx, path), stbuf);
> if (err) {
> - return err;
> + return -errno;
> }
> if (fs_ctx->fs_sm == SM_MAPPED) {
> /* Actual credentials are part of extended attrs */
> @@ -51,7 +51,7 @@ static int local_lstat(FsContext *fs_ctx,
> const char *path, struct stat *stbuf)
> stbuf->st_rdev = tmp_dev;
> }
> }
> - return err;
> + return 0;
> }
>
> static int local_set_xattr(const char *path, FsCred *credp)
> @@ -61,28 +61,28 @@ static int local_set_xattr(const char
> *path, FsCred *credp)
> err = setxattr(path,
> "user.virtfs.uid",&credp->fc_uid, sizeof(uid_t),
> 0);
> if (err) {
> - return err;
> + return -errno;
> }
> }
> if (credp->fc_gid != -1) {
> err = setxattr(path,
> "user.virtfs.gid",&credp->fc_gid, sizeof(gid_t),
> 0);
> if (err) {
> - return err;
> + return -errno;
> }
> }
> if (credp->fc_mode != -1) {
> err = setxattr(path, "user.virtfs.mode",&credp->fc_mode,
> sizeof(mode_t), 0);
> if (err) {
> - return err;
> + return -errno;
> }
> }
> if (credp->fc_rdev != -1) {
> err = setxattr(path, "user.virtfs.rdev",&credp->fc_rdev,
> sizeof(dev_t), 0);
> if (err) {
> - return err;
> + return -errno;
> }
> }
> return 0;
> @@ -92,7 +92,7 @@ static int
> local_post_create_passthrough(FsContext *fs_ctx, const char *path,
> FsCred *credp)
> {
> if (chmod(rpath(fs_ctx, path), credp->fc_mode& 07777)< 0) {
> - return -1;
> + return -errno;
> }
> if (lchown(rpath(fs_ctx, path), credp->fc_uid,
> credp->fc_gid)< 0) {
> /*
> @@ -100,7 +100,7 @@ static int
> local_post_create_passthrough(FsContext *fs_ctx, const char *path,
> * using security model none. Ignore the error
> */
> if (fs_ctx->fs_sm != SM_NONE) {
> - return -1;
> + return -errno;
> }
> }
> return 0;
> @@ -114,38 +114,40 @@ static ssize_t local_readlink(FsContext
> *fs_ctx, const char *path,
> int fd;
> fd = open(rpath(fs_ctx, path), O_RDONLY);
> if (fd == -1) {
> - return -1;
> + return -errno;
> }
> do {
> tsize = read(fd, (void *)buf, bufsz);
> } while (tsize == -1&& errno == EINTR);
> close(fd);
> - return tsize;
> + return tsize>= 0 ? tsize : -errno;
>
tsize >= 0 ? tsize..... Many places missing proper spaces.
Please follow the coding guide lines.
> } else if ((fs_ctx->fs_sm == SM_PASSTHROUGH) ||
> (fs_ctx->fs_sm == SM_NONE)) {
> tsize = readlink(rpath(fs_ctx, path), buf, bufsz);
> }
> - return tsize;
> + return tsize>= 0 ? tsize : -errno;
> }
>
> static int local_close(FsContext *ctx, int fd)
> {
> - return close(fd);
> + return close(fd) ? -errno : 0;
> }
>
> static int local_closedir(FsContext *ctx, DIR *dir)
> {
> - return closedir(dir);
> + return closedir(dir) ? -errno : 0;
> }
>
> static int local_open(FsContext *ctx, const char *path, int
> flags)
> {
> - return open(rpath(ctx, path), flags);
> + int ret = open(rpath(ctx, path), flags);
> + return ret>= 0 ? ret : -errno;
> }
>
> -static DIR *local_opendir(FsContext *ctx, const char *path)
> +static int local_opendir(FsContext *ctx, const char *path,
> DIR **dir)
> {
> - return opendir(rpath(ctx, path));
> + *dir = opendir(rpath(ctx, path));
> + return *dir ? 0 : -errno;
> }
>
> static void local_rewinddir(FsContext *ctx, DIR *dir)
> @@ -155,12 +157,15 @@ static void local_rewinddir(FsContext
> *ctx, DIR *dir)
>
> static off_t local_telldir(FsContext *ctx, DIR *dir)
> {
> - return telldir(dir);
> + int ret = telldir(dir);
> + return ret>= 0 ? ret : -errno;
> }
>
> -static struct dirent *local_readdir(FsContext *ctx, DIR *dir)
> +static int local_readdir(FsContext *ctx, DIR *dir, struct
> dirent **dirent)
> {
> - return readdir(dir);
> + *dirent = readdir(dir);
> + return dirent ? 0 : -errno;
>
*dirent ? 0 : -errno;
> +
> }
>
> static void local_seekdir(FsContext *ctx, DIR *dir, off_t off)
> @@ -171,14 +176,17 @@ static void local_seekdir(FsContext
> *ctx, DIR *dir, off_t off)
> static ssize_t local_preadv(FsContext *ctx, int fd, const
> struct iovec *iov,
> int iovcnt, off_t offset)
> {
> + int err;
> #ifdef CONFIG_PREADV
> - return preadv(fd, iov, iovcnt, offset);
> + err = preadv(fd, iov, iovcnt, offset);
> + return err>= 0 ? err : -errno;
> #else
> int err = lseek(fd, offset, SEEK_SET);
> if (err == -1) {
> - return err;
> + return -errno;
> } else {
> - return readv(fd, iov, iovcnt);
> + err = readv(fd, iov, iovcnt);
> + return err>= 0 ? err : -errno;
> }
> #endif
> }
> @@ -186,27 +194,32 @@ static ssize_t local_preadv(FsContext
> *ctx, int fd, const struct iovec *iov,
> static ssize_t local_pwritev(FsContext *ctx, int fd, const
> struct iovec *iov,
> int iovcnt, off_t offset)
> {
> + int err;
> #ifdef CONFIG_PREADV
> - return pwritev(fd, iov, iovcnt, offset);
> + err = pwritev(fd, iov, iovcnt, offset);
> + return err>= 0 ? err : -errno;
> #else
> int err = lseek(fd, offset, SEEK_SET);
> if (err == -1) {
> - return err;
> + return -errno;
> } else {
> - return writev(fd, iov, iovcnt);
> + err = writev(fd, iov, iovcnt);
> + return err>= 0 ? err : -errno;
> }
> #endif
> }
>
> static int local_chmod(FsContext *fs_ctx, const char *path,
> FsCred *credp)
> {
> + int ret;
> if (fs_ctx->fs_sm == SM_MAPPED) {
> return local_set_xattr(rpath(fs_ctx, path), credp);
> } else if ((fs_ctx->fs_sm == SM_PASSTHROUGH) ||
> (fs_ctx->fs_sm == SM_NONE)) {
> - return chmod(rpath(fs_ctx, path), credp->fc_mode);
> + ret = chmod(rpath(fs_ctx, path), credp->fc_mode);
> + return ret>= 0 ? ret : -errno;
>
Why >= here?
ret ? -errno : 0;
or tp be specific
ret == -1 ? -errno: 0;
> }
> - return -1;
> + return -ENOTSUP;
> }
>
> static int local_mknod(FsContext *fs_ctx, const char *path,
> FsCred *credp)
> @@ -218,7 +231,7 @@ static int local_mknod(FsContext *fs_ctx,
> const char *path, FsCred *credp)
> if (fs_ctx->fs_sm == SM_MAPPED) {
> err = mknod(rpath(fs_ctx, path),
> SM_LOCAL_MODE_BITS|S_IFREG, 0);
> if (err == -1) {
> - return err;
> + return -errno;
> }
> local_set_xattr(rpath(fs_ctx, path), credp);
> if (err == -1) {
> @@ -229,7 +242,7 @@ static int local_mknod(FsContext *fs_ctx,
> const char *path, FsCred *credp)
> (fs_ctx->fs_sm == SM_NONE)) {
> err = mknod(rpath(fs_ctx, path), credp->fc_mode,
> credp->fc_rdev);
> if (err == -1) {
> - return err;
> + return -errno;
> }
> err = local_post_create_passthrough(fs_ctx, path, credp);
> if (err == -1) {
> @@ -237,12 +250,12 @@ static int local_mknod(FsContext
> *fs_ctx, const char *path, FsCred *credp)
> goto err_end;
> }
> }
> - return err;
> + return 0;
>
> err_end:
> remove(rpath(fs_ctx, path));
> errno = serrno;
> - return err;
> + return -errno;
> }
>
> static int local_mkdir(FsContext *fs_ctx, const char *path,
> FsCred *credp)
> @@ -254,7 +267,7 @@ static int local_mkdir(FsContext *fs_ctx,
> const char *path, FsCred *credp)
> if (fs_ctx->fs_sm == SM_MAPPED) {
> err = mkdir(rpath(fs_ctx, path), SM_LOCAL_DIR_MODE_BITS);
> if (err == -1) {
> - return err;
> + return -errno;
> }
> credp->fc_mode = credp->fc_mode|S_IFDIR;
> err = local_set_xattr(rpath(fs_ctx, path), credp);
> @@ -266,7 +279,7 @@ static int local_mkdir(FsContext *fs_ctx,
> const char *path, FsCred *credp)
> (fs_ctx->fs_sm == SM_NONE)) {
> err = mkdir(rpath(fs_ctx, path), credp->fc_mode);
> if (err == -1) {
> - return err;
> + return -errno;
> }
> err = local_post_create_passthrough(fs_ctx, path, credp);
> if (err == -1) {
> @@ -274,12 +287,12 @@ static int local_mkdir(FsContext
> *fs_ctx, const char *path, FsCred *credp)
> goto err_end;
> }
> }
> - return err;
> + return 0;
>
> err_end:
> remove(rpath(fs_ctx, path));
> errno = serrno;
> - return err;
> + return -errno;
> }
>
> static int local_fstat(FsContext *fs_ctx, int fd, struct stat
> *stbuf)
> @@ -287,7 +300,7 @@ static int local_fstat(FsContext *fs_ctx,
> int fd, struct stat *stbuf)
> int err;
> err = fstat(fd, stbuf);
> if (err) {
> - return err;
> + return -errno;
> }
> if (fs_ctx->fs_sm == SM_MAPPED) {
> /* Actual credentials are part of extended attrs */
> @@ -309,7 +322,7 @@ static int local_fstat(FsContext *fs_ctx,
> int fd, struct stat *stbuf)
> stbuf->st_rdev = tmp_dev;
> }
> }
> - return err;
> + return 0;
> }
>
> static int local_open2(FsContext *fs_ctx, const char *path,
> int flags,
> @@ -323,7 +336,7 @@ static int local_open2(FsContext *fs_ctx,
> const char *path, int flags,
> if (fs_ctx->fs_sm == SM_MAPPED) {
> fd = open(rpath(fs_ctx, path), flags,
> SM_LOCAL_MODE_BITS);
> if (fd == -1) {
> - return fd;
> + return -errno;
> }
> credp->fc_mode = credp->fc_mode|S_IFREG;
> /* Set cleint credentials in xattr */
> @@ -336,7 +349,7 @@ static int local_open2(FsContext *fs_ctx,
> const char *path, int flags,
> (fs_ctx->fs_sm == SM_NONE)) {
> fd = open(rpath(fs_ctx, path), flags, credp->fc_mode);
> if (fd == -1) {
> - return fd;
> + return -errno;
> }
> err = local_post_create_passthrough(fs_ctx, path, credp);
> if (err == -1) {
> @@ -350,7 +363,7 @@ err_end:
> close(fd);
> remove(rpath(fs_ctx, path));
> errno = serrno;
> - return err;
> + return -errno;
> }
>
>
> @@ -367,7 +380,7 @@ static int local_symlink(FsContext
> *fs_ctx, const char *oldpath,
> fd = open(rpath(fs_ctx, newpath), O_CREAT|O_EXCL|O_RDWR,
> SM_LOCAL_MODE_BITS);
> if (fd == -1) {
> - return fd;
> + return -errno;
> }
> /* Write the oldpath (target) to the file. */
> oldpath_size = strlen(oldpath);
> @@ -393,7 +406,7 @@ static int local_symlink(FsContext
> *fs_ctx, const char *oldpath,
> (fs_ctx->fs_sm == SM_NONE)) {
> err = symlink(oldpath, rpath(fs_ctx, newpath));
> if (err) {
> - return err;
> + return -errno;
> }
> err = lchown(rpath(fs_ctx, newpath), credp->fc_uid,
> credp->fc_gid);
> if (err == -1) {
> @@ -408,12 +421,12 @@ static int local_symlink(FsContext
> *fs_ctx, const char *oldpath,
> err = 0;
> }
> }
> - return err;
> + return 0;
>
> err_end:
> remove(rpath(fs_ctx, newpath));
> errno = serrno;
> - return err;
> + return -errno;
> }
>
> static int local_link(FsContext *ctx, const char *oldpath,
> const char *newpath)
> @@ -436,12 +449,12 @@ static int local_link(FsContext *ctx,
> const char *oldpath, const char *newpath)
> errno = serrno;
> }
>
> - return err;
> + return err>= 0 ? err : -errno;
> }
>
> static int local_truncate(FsContext *ctx, const char *path,
> off_t size)
> {
> - return truncate(rpath(ctx, path), size);
> + return truncate(rpath(ctx, path), size) ? -errno : 0;
> }
>
> static int local_rename(FsContext *ctx, const char *oldpath,
> @@ -461,7 +474,7 @@ static int local_rename(FsContext *ctx,
> const char *oldpath,
> qemu_free(tmp);
> }
>
> - return err;
> + return err>= 0 ? err : -errno;
>
> }
>
> @@ -469,14 +482,14 @@ static int local_chown(FsContext
> *fs_ctx, const char *path, FsCred *credp)
> {
> if ((credp->fc_uid == -1&& credp->fc_gid == -1) ||
> (fs_ctx->fs_sm == SM_PASSTHROUGH)) {
> - return lchown(rpath(fs_ctx, path), credp->fc_uid,
> credp->fc_gid);
> + return lchown(rpath(fs_ctx, path), credp->fc_uid,
> credp->fc_gid) ? -errno : 0;
> } else if (fs_ctx->fs_sm == SM_MAPPED) {
> return local_set_xattr(rpath(fs_ctx, path), credp);
> } else if ((fs_ctx->fs_sm == SM_PASSTHROUGH) ||
> (fs_ctx->fs_sm == SM_NONE)) {
> - return lchown(rpath(fs_ctx, path), credp->fc_uid,
> credp->fc_gid);
> + return lchown(rpath(fs_ctx, path), credp->fc_uid,
> credp->fc_gid) ? -errno : 0;
> }
> - return -1;
> + return -EINVAL;
> }
>
> static int local_utimensat(FsContext *s, const char *path,
> @@ -487,21 +500,21 @@ static int local_utimensat(FsContext *s,
> const char *path,
>
> static int local_remove(FsContext *ctx, const char *path)
> {
> - return remove(rpath(ctx, path));
> + return remove(rpath(ctx, path)) ? -errno : 0;
> }
>
> static int local_fsync(FsContext *ctx, int fd, int datasync)
> {
> if (datasync) {
> - return qemu_fdatasync(fd);
> + return qemu_fdatasync(fd) ? -errno : 0;
> } else {
> - return fsync(fd);
> + return fsync(fd) ? -errno : 0;
> }
> }
>
> static int local_statfs(FsContext *s, const char *path,
> struct statfs *stbuf)
> {
> - return statfs(rpath(s, path), stbuf);
> + return statfs(rpath(s, path), stbuf) ? - errno : 0;
> }
>
> static ssize_t local_lgetxattr(FsContext *ctx, const char *path,
> diff --git a/hw/9pfs/virtio-9p-xattr.c b/hw/9pfs/virtio-9p-xattr.c
> index 03c3d3f..b37ea44 100644
> --- a/hw/9pfs/virtio-9p-xattr.c
> +++ b/hw/9pfs/virtio-9p-xattr.c
> @@ -32,9 +32,14 @@ static XattrOperations
> *get_xattr_operations(XattrOperations **h,
> ssize_t v9fs_get_xattr(FsContext *ctx, const char *path,
> const char *name, void *value, size_t
> size)
> {
> + int ret;
> XattrOperations *xops = get_xattr_operations(ctx->xops,
> name);
> if (xops) {
> - return xops->getxattr(ctx, path, name, value, size);
> + ret = xops->getxattr(ctx, path, name, value, size);
> + if (ret< 0) {
> + return -errno;
> + }
> + return ret;
> }
> errno = -EOPNOTSUPP;
> return -1;
> @@ -117,9 +122,14 @@ err_out:
> int v9fs_set_xattr(FsContext *ctx, const char *path, const
> char *name,
> void *value, size_t size, int flags)
> {
> + int ret;
> XattrOperations *xops = get_xattr_operations(ctx->xops,
> name);
> if (xops) {
> - return xops->setxattr(ctx, path, name, value, size,
> flags);
> + ret = xops->setxattr(ctx, path, name, value, size,
> flags);
> + if (ret< 0) {
> + return -errno;
> + }
> + return ret;
> }
> errno = -EOPNOTSUPP;
> return -1;
> @@ -129,9 +139,14 @@ int v9fs_set_xattr(FsContext *ctx, const
> char *path, const char *name,
> int v9fs_remove_xattr(FsContext *ctx,
> const char *path, const char *name)
> {
> + int ret;
> XattrOperations *xops = get_xattr_operations(ctx->xops,
> name);
> if (xops) {
> - return xops->removexattr(ctx, path, name);
> + ret = xops->removexattr(ctx, path, name);
> + if (ret< 0) {
> + return -errno;
> + }
> + return ret;
> }
> errno = -EOPNOTSUPP;
> return -1;
> diff --git a/hw/9pfs/virtio-9p.c b/hw/9pfs/virtio-9p.c
> index 9d72021..374a766 100644
> --- a/hw/9pfs/virtio-9p.c
> +++ b/hw/9pfs/virtio-9p.c
> @@ -110,9 +110,9 @@ static int v9fs_do_open(V9fsState *s,
> V9fsString *path, int flags)
> return s->ops->open(&s->ctx, path->data, flags);
> }
>
> -static DIR *v9fs_do_opendir(V9fsState *s, V9fsString *path)
> +static int v9fs_do_opendir(V9fsState *s, V9fsString *path,
> DIR **dir)
> {
> - return s->ops->opendir(&s->ctx, path->data);
> + return s->ops->opendir(&s->ctx, path->data, dir);
> }
>
> static void v9fs_do_rewinddir(V9fsState *s, DIR *dir)
> @@ -125,9 +125,9 @@ static off_t v9fs_do_telldir(V9fsState *s,
> DIR *dir)
> return s->ops->telldir(&s->ctx, dir);
> }
>
> -static struct dirent *v9fs_do_readdir(V9fsState *s, DIR *dir)
> +static int v9fs_do_readdir(V9fsState *s, DIR *dir, struct
> dirent **dirent)
> {
> - return s->ops->readdir(&s->ctx, dir);
> + return s->ops->readdir(&s->ctx, dir, dirent);
> }
>
> static void v9fs_do_seekdir(V9fsState *s, DIR *dir, off_t off)
> @@ -1038,8 +1038,7 @@ static int stat_to_v9stat(V9fsState *s,
> V9fsString *name,
>
> if (v9stat->mode& P9_STAT_MODE_SYMLINK) {
> err = v9fs_do_readlink(s, name,&v9stat->extension);
> - if (err == -1) {
> - err = -errno;
> + if (err< 0) {
> return err;
> }
> v9stat->extension.data[err] = 0;
> @@ -1234,8 +1233,7 @@ out:
>
> static void v9fs_stat_post_lstat(V9fsState *s, V9fsStatState
> *vs, int err)
> {
> - if (err == -1) {
> - err = -errno;
> + if (err< 0) {
> goto out;
> }
>
> @@ -1263,9 +1261,8 @@ static void v9fs_stat(V9fsState *s,
> V9fsPDU *pdu)
> vs->offset = 7;
>
> memset(&vs->v9stat, 0, sizeof(vs->v9stat));
> -
> pdu_unmarshal(vs->pdu, vs->offset, "d",&fid);
> -
> +
> vs->fidp = lookup_fid(s, fid);
> if (vs->fidp == NULL) {
> err = -ENOENT;
> @@ -1285,8 +1282,7 @@ out:
> static void v9fs_getattr_post_lstat(V9fsState *s,
> V9fsStatStateDotl *vs,
>
> int err)
> {
> - if (err == -1) {
> - err = -errno;
> + if (err< 0) {
> goto out;
> }
>
> @@ -1348,8 +1344,7 @@ out:
> static void v9fs_setattr_post_truncate(V9fsState *s,
> V9fsSetattrState *vs,
>
> int err)
> {
> - if (err == -1) {
> - err = -errno;
> + if (err< 0) {
> goto out;
> }
> err = vs->offset;
> @@ -1361,8 +1356,7 @@ out:
>
> static void v9fs_setattr_post_chown(V9fsState *s,
> V9fsSetattrState *vs, int err)
> {
> - if (err == -1) {
> - err = -errno;
> + if (err< 0) {
> goto out;
> }
>
> @@ -1380,8 +1374,7 @@ out:
> static void v9fs_setattr_post_utimensat(V9fsState *s,
> V9fsSetattrState *vs,
>
> int err)
> {
> - if (err == -1) {
> - err = -errno;
> + if (err< 0) {
> goto out;
> }
>
> @@ -1410,8 +1403,7 @@ out:
>
> static void v9fs_setattr_post_chmod(V9fsState *s,
> V9fsSetattrState *vs, int err)
> {
> - if (err == -1) {
> - err = -errno;
> + if (err< 0) {
> goto out;
> }
>
> @@ -1506,7 +1498,7 @@ static void
> v9fs_walk_marshal(V9fsWalkState *vs)
> static void v9fs_walk_post_newfid_lstat(V9fsState *s,
> V9fsWalkState *vs,
>
> int err)
> {
> - if (err == -1) {
> + if (err< 0) {
> free_fid(s, vs->newfidp->fid);
> v9fs_string_free(&vs->path);
> err = -ENOENT;
> @@ -1536,7 +1528,7 @@ out:
> static void v9fs_walk_post_oldfid_lstat(V9fsState *s,
> V9fsWalkState *vs,
> int err)
> {
> - if (err == -1) {
> + if (err< 0) {
> v9fs_string_free(&vs->path);
> err = -ENOENT;
> goto out;
> @@ -1665,8 +1657,7 @@ static int32_t get_iounit(V9fsState *s,
> V9fsString *name)
>
> static void v9fs_open_post_opendir(V9fsState *s,
> V9fsOpenState *vs, int err)
> {
> - if (vs->fidp->fs.dir == NULL) {
> - err = -errno;
> + if (err< 0) {
> goto out;
> }
> vs->fidp->fid_type = P9_FID_DIR;
> @@ -1689,8 +1680,7 @@ static void
> v9fs_open_post_getiounit(V9fsState *s, V9fsOpenState *vs)
>
> static void v9fs_open_post_open(V9fsState *s, V9fsOpenState
> *vs, int err)
> {
> - if (vs->fidp->fs.fd == -1) {
> - err = -errno;
> + if (vs->fidp->fs.fd< 0) {
>
where are you setting err.?
err = vs->fidp->fs.fd ?
>
> goto out;
> }
> vs->fidp->fid_type = P9_FID_FILE;
> @@ -1707,14 +1697,13 @@ static void
> v9fs_open_post_lstat(V9fsState *s, V9fsOpenState *vs, int err)
> int flags;
>
> if (err) {
> - err = -errno;
> goto out;
> }
>
> stat_to_qid(&vs->stbuf,&vs->qid);
>
> if (S_ISDIR(vs->stbuf.st_mode)) {
> - vs->fidp->fs.dir = v9fs_do_opendir(s,&vs->fidp->path);
> + err =
> v9fs_do_opendir(s,&vs->fidp->path,&vs->fidp->fs.dir);
> v9fs_open_post_opendir(s, vs, err);
> } else {
> if (s->proto_version == V9FS_PROTO_2000L) {
> @@ -1778,7 +1767,6 @@ static void v9fs_post_lcreate(V9fsState
> *s, V9fsLcreateState *vs, int err)
> err = vs->offset;
> } else {
> vs->fidp->fid_type = P9_FID_NONE;
> - err = -errno;
> if (vs->fidp->fs.fd> 0) {
> close(vs->fidp->fs.fd);
> }
> @@ -1794,7 +1782,6 @@ static void
> v9fs_lcreate_post_get_iounit(V9fsState *s, V9fsLcreateState *vs,
> int err)
> {
> if (err) {
> - err = -errno;
> goto out;
> }
> err = v9fs_do_lstat(s,&vs->fullname,&vs->stbuf);
> @@ -1806,8 +1793,7 @@ out:
> static void v9fs_lcreate_post_do_open2(V9fsState *s,
> V9fsLcreateState *vs,
> int err)
> {
> - if (vs->fidp->fs.fd == -1) {
> - err = -errno;
> + if (vs->fidp->fs.fd< 0) {
> goto out;
>
err?
> }
> vs->fidp->fid_type = P9_FID_FILE;
> @@ -1860,9 +1846,6 @@ out:
>
> static void v9fs_post_do_fsync(V9fsState *s, V9fsPDU *pdu,
> int err)
> {
> - if (err == -1) {
> - err = -errno;
> - }
> complete_pdu(s, pdu, err);
> }
>
> @@ -1932,7 +1915,6 @@ static void
> v9fs_read_post_dir_lstat(V9fsState *s, V9fsReadState *vs,
> ssize_t err)
> {
> if (err) {
> - err = -errno;
> goto out;
> }
> err = stat_to_v9stat(s,&vs->name,&vs->stbuf,&vs->v9stat);
> @@ -1952,7 +1934,7 @@ static void
> v9fs_read_post_dir_lstat(V9fsState *s, V9fsReadState *vs,
> v9fs_stat_free(&vs->v9stat);
> v9fs_string_free(&vs->name);
> vs->dir_pos = vs->dent->d_off;
> - vs->dent = v9fs_do_readdir(s, vs->fidp->fs.dir);
> + err = v9fs_do_readdir(s, vs->fidp->fs.dir,&vs->dent);
> v9fs_read_post_readdir(s, vs, err);
> return;
> out:
> @@ -1984,7 +1966,7 @@ static void
> v9fs_read_post_readdir(V9fsState *s, V9fsReadState *vs,
> ssize_t err)
>
> static void v9fs_read_post_telldir(V9fsState *s,
> V9fsReadState *vs, ssize_t err)
> {
> - vs->dent = v9fs_do_readdir(s, vs->fidp->fs.dir);
> + v9fs_do_readdir(s, vs->fidp->fs.dir,&vs->dent);
>
space after ,
Please go through the coding guide lines.
Thanks,
JV
> v9fs_read_post_readdir(s, vs, err);
> return;
> }
> @@ -2000,8 +1982,6 @@ static void
> v9fs_read_post_rewinddir(V9fsState *s, V9fsReadState *vs,
> static void v9fs_read_post_preadv(V9fsState *s, V9fsReadState
> *vs, ssize_t err)
> {
> if (err< 0) {
> - /* IO error return the error */
> - err = -errno;
> goto out;
> }
> vs->total += vs->len;
> @@ -2017,9 +1997,6 @@ static void
> v9fs_read_post_preadv(V9fsState *s, V9fsReadState *vs, ssize_t
> err)
> vs->off += vs->len;
> }
> } while (vs->len == -1&& errno == EINTR);
> - if (vs->len == -1) {
> - err = -errno;
> - }
> v9fs_read_post_preadv(s, vs, err);
> return;
> }
> @@ -2170,7 +2147,7 @@ static void
> v9fs_readdir_post_readdir(V9fsState *s, V9fsReadDirState *vs)
> vs->count += len;
> v9fs_string_free(&vs->name);
> vs->saved_dir_pos = vs->dent->d_off;
> - vs->dent = v9fs_do_readdir(s, vs->fidp->fs.dir);
> + v9fs_do_readdir(s, vs->fidp->fs.dir,&vs->dent);
> v9fs_readdir_post_readdir(s, vs);
> return;
> }
> @@ -2184,7 +2161,7 @@ static void
> v9fs_readdir_post_readdir(V9fsState *s, V9fsReadDirState *vs)
>
> static void v9fs_readdir_post_telldir(V9fsState *s,
> V9fsReadDirState *vs)
> {
> - vs->dent = v9fs_do_readdir(s, vs->fidp->fs.dir);
> + v9fs_do_readdir(s, vs->fidp->fs.dir,&vs->dent);
> v9fs_readdir_post_readdir(s, vs);
> return;
> }
> @@ -2236,8 +2213,6 @@ static void
> v9fs_write_post_pwritev(V9fsState *s, V9fsWriteState *vs,
> ssize_t err)
> {
> if (err< 0) {
> - /* IO error return the error */
> - err = -errno;
> goto out;
> }
> vs->total += vs->len;
> @@ -2253,9 +2228,6 @@ static void
> v9fs_write_post_pwritev(V9fsState *s, V9fsWriteState *vs,
> vs->off += vs->len;
> }
> } while (vs->len == -1&& errno == EINTR);
> - if (vs->len == -1) {
> - err = -errno;
> - }
> v9fs_write_post_pwritev(s, vs, err);
> return;
> }
> @@ -2394,18 +2366,12 @@ static void v9fs_post_create(V9fsState
> *s, V9fsCreateState *vs, int err)
>
> static void v9fs_create_post_perms(V9fsState *s,
> V9fsCreateState *vs, int err)
> {
> - if (err) {
> - err = -errno;
> - }
> v9fs_post_create(s, vs, err);
> }
>
> static void v9fs_create_post_opendir(V9fsState *s,
> V9fsCreateState *vs,
>
> int err)
> {
> - if (!vs->fidp->fs.dir) {
> - err = -errno;
> - }
> vs->fidp->fid_type = P9_FID_DIR;
> v9fs_post_create(s, vs, err);
> }
> @@ -2414,11 +2380,10 @@ static void
> v9fs_create_post_dir_lstat(V9fsState *s, V9fsCreateState *vs,
>
> int err)
> {
> if (err) {
> - err = -errno;
> goto out;
> }
>
> - vs->fidp->fs.dir = v9fs_do_opendir(s,&vs->fullname);
> + err = v9fs_do_opendir(s,&vs->fullname,&vs->fidp->fs.dir);
> v9fs_create_post_opendir(s, vs, err);
> return;
>
> @@ -2429,7 +2394,6 @@ out:
> static void v9fs_create_post_mkdir(V9fsState *s,
> V9fsCreateState *vs, int err)
> {
> if (err) {
> - err = -errno;
> goto out;
> }
>
> @@ -2446,7 +2410,6 @@ static void
> v9fs_create_post_fstat(V9fsState *s, V9fsCreateState *vs, int err)
> if (err) {
> vs->fidp->fid_type = P9_FID_NONE;
> close(vs->fidp->fs.fd);
> - err = -errno;
> }
> v9fs_post_create(s, vs, err);
> return;
> @@ -2455,7 +2418,6 @@ static void
> v9fs_create_post_fstat(V9fsState *s, V9fsCreateState *vs, int err)
> static void v9fs_create_post_open2(V9fsState *s,
> V9fsCreateState *vs, int err)
> {
> if (vs->fidp->fs.fd == -1) {
> - err = -errno;
> goto out;
> }
> vs->fidp->fid_type = P9_FID_FILE;
> @@ -2472,8 +2434,7 @@ out:
> static void v9fs_create_post_lstat(V9fsState *s,
> V9fsCreateState *vs, int err)
> {
>
> - if (err == 0 || errno != ENOENT) {
> - err = -errno;
> + if (err == 0 || err != -ENOENT) {
> goto out;
> }
>
> @@ -2501,7 +2462,6 @@ static void
> v9fs_create_post_lstat(V9fsState *s, V9fsCreateState *vs, int err)
>
> if (sscanf(vs->extension.data, "%c %u %u",&ctype,&major,
> &minor) != 3) {
> - err = -errno;
> v9fs_post_create(s, vs, err);
> }
>
> @@ -2583,8 +2543,6 @@ static void v9fs_post_symlink(V9fsState
> *s, V9fsSymlinkState *vs, int err)
> stat_to_qid(&vs->stbuf,&vs->qid);
> vs->offset += pdu_marshal(vs->pdu, vs->offset,
> "Q",&vs->qid);
> err = vs->offset;
> - } else {
> - err = -errno;
> }
> complete_pdu(s, vs->pdu, err);
> v9fs_string_free(&vs->name);
> @@ -2660,22 +2618,17 @@ static void v9fs_link(V9fsState *s,
> V9fsPDU *pdu)
>
> dfidp = lookup_fid(s, dfid);
> if (dfidp == NULL) {
> - err = -errno;
> goto out;
> }
>
> oldfidp = lookup_fid(s, oldfid);
> if (oldfidp == NULL) {
> - err = -errno;
> goto out;
> }
>
> v9fs_string_sprintf(&fullname, "%s/%s", dfidp->path.data,
> name.data);
> err = offset;
> err = v9fs_do_link(s,&oldfidp->path,&fullname);
> - if (err) {
> - err = -errno;
> - }
> v9fs_string_free(&fullname);
>
> out:
> @@ -2686,9 +2639,7 @@ out:
> static void v9fs_remove_post_remove(V9fsState *s,
> V9fsRemoveState *vs,
>
> int err)
> {
> - if (err< 0) {
> - err = -errno;
> - } else {
> + if (err == 0) {
> err = vs->offset;
> }
>
> @@ -2746,9 +2697,7 @@ static void
> v9fs_wstat_post_rename(V9fsState *s, V9fsWstatState *vs, int err)
> goto out;
> }
> if (vs->v9stat.length != -1) {
> - if (v9fs_do_truncate(s,&vs->fidp->path,
> vs->v9stat.length)< 0) {
> - err = -errno;
> - }
> + err = v9fs_do_truncate(s,&vs->fidp->path,
> vs->v9stat.length);
> }
> v9fs_wstat_post_truncate(s, vs, err);
> return;
> @@ -2800,9 +2749,8 @@ static int
> v9fs_complete_rename(V9fsState *s, V9fsRenameState *vs)
> vs->name.size = strlen(new_name);
>
> if (strcmp(new_name, vs->fidp->path.data) != 0) {
> - if (v9fs_do_rename(s,&vs->fidp->path,&vs->name)) {
> - err = -errno;
> - } else {
> + err = v9fs_do_rename(s,&vs->fidp->path,&vs->name);
> + if (!err) {
> V9fsFidState *fidp;
> /*
> * Fixup fid's pointing to the old name to
> @@ -2908,10 +2856,8 @@ static void
> v9fs_wstat_post_utime(V9fsState *s, V9fsWstatState *vs, int err)
> }
>
> if (vs->v9stat.n_gid != -1 || vs->v9stat.n_uid != -1) {
> - if (v9fs_do_chown(s,&vs->fidp->path, vs->v9stat.n_uid,
> - vs->v9stat.n_gid)) {
> - err = -errno;
> - }
> + err = v9fs_do_chown(s,&vs->fidp->path, vs->v9stat.n_uid,
> + vs->v9stat.n_gid);
> }
> v9fs_wstat_post_chown(s, vs, err);
> return;
> @@ -2943,9 +2889,7 @@ static void
> v9fs_wstat_post_chmod(V9fsState *s, V9fsWstatState *vs, int err)
> times[1].tv_nsec = UTIME_OMIT;
> }
>
> - if (v9fs_do_utimensat(s,&vs->fidp->path, times)) {
> - err = -errno;
> - }
> + err = v9fs_do_utimensat(s,&vs->fidp->path, times);
> }
>
> v9fs_wstat_post_utime(s, vs, err);
> @@ -2959,9 +2903,6 @@ out:
>
> static void v9fs_wstat_post_fsync(V9fsState *s,
> V9fsWstatState *vs, int err)
> {
> - if (err == -1) {
> - err = -errno;
> - }
> v9fs_stat_free(&vs->v9stat);
> complete_pdu(s, vs->pdu, err);
> qemu_free(vs);
> @@ -2971,8 +2912,7 @@ static void
> v9fs_wstat_post_lstat(V9fsState *s, V9fsWstatState *vs, int err)
> {
> uint32_t v9_mode;
>
> - if (err == -1) {
> - err = -errno;
> + if (err< 0) {
> goto out;
> }
>
> @@ -2985,10 +2925,8 @@ static void
> v9fs_wstat_post_lstat(V9fsState *s, V9fsWstatState *vs, int err)
> goto out;
> }
>
> - if (v9fs_do_chmod(s,&vs->fidp->path,
> v9mode_to_mode(vs->v9stat.mode,
> -&vs->v9stat.extension))) {
> - err = -errno;
> - }
> + err = v9fs_do_chmod(s,&vs->fidp->path,
> v9mode_to_mode(vs->v9stat.mode,
> +&vs->v9stat.extension));
> v9fs_wstat_post_chmod(s, vs, err);
> return;
>
> @@ -3049,7 +2987,6 @@ static void
> v9fs_statfs_post_statfs(V9fsState *s, V9fsStatfsState *vs, int
> err)
> int32_t bsize_factor;
>
> if (err) {
> - err = -errno;
> goto out;
> }
>
> @@ -3119,8 +3056,7 @@ out:
>
> static void v9fs_mknod_post_lstat(V9fsState *s, V9fsMkState
> *vs, int err)
> {
> - if (err == -1) {
> - err = -errno;
> + if (err< 0) {
> goto out;
> }
>
> @@ -3136,8 +3072,7 @@ out:
>
> static void v9fs_mknod_post_mknod(V9fsState *s, V9fsMkState
> *vs, int err)
> {
> - if (err == -1) {
> - err = -errno;
> + if (err< 0) {
> goto out;
> }
>
> @@ -3234,7 +3169,6 @@ static void v9fs_lock(V9fsState *s,
> V9fsPDU *pdu)
> goto out;
> }
> if (err< 0) {
> - err = -errno;
> goto out;
> }
> vs->status = P9_LOCK_SUCCESS;
> @@ -3280,7 +3214,6 @@ static void v9fs_getlock(V9fsState *s,
> V9fsPDU *pdu)
> goto out;
> }
> if (err< 0) {
> - err = -errno;
> goto out;
> }
> vs->glock->type = F_UNLCK;
> @@ -3295,8 +3228,7 @@ out:
>
> static void v9fs_mkdir_post_lstat(V9fsState *s, V9fsMkState
> *vs, int err)
> {
> - if (err == -1) {
> - err = -errno;
> + if (err< 0) {
> goto out;
> }
>
> @@ -3312,8 +3244,7 @@ out:
>
> static void v9fs_mkdir_post_mkdir(V9fsState *s, V9fsMkState
> *vs, int err)
> {
> - if (err == -1) {
> - err = -errno;
> + if (err< 0) {
> goto out;
> }
>
> @@ -3366,7 +3297,6 @@ static void
> v9fs_post_xattr_getvalue(V9fsState *s, V9fsXattrState *vs, int
> err)
> {
>
> if (err< 0) {
> - err = -errno;
> free_fid(s, vs->xattr_fidp->fid);
> goto out;
> }
> @@ -3382,7 +3312,6 @@ out:
> static void v9fs_post_xattr_check(V9fsState *s,
> V9fsXattrState *vs, ssize_t err)
> {
> if (err< 0) {
> - err = -errno;
> free_fid(s, vs->xattr_fidp->fid);
> goto out;
> }
> @@ -3410,7 +3339,6 @@ static void
> v9fs_post_lxattr_getvalue(V9fsState *s,
> V9fsXattrState *vs, int
> err)
> {
> if (err< 0) {
> - err = -errno;
> free_fid(s, vs->xattr_fidp->fid);
> goto out;
> }
> @@ -3427,7 +3355,6 @@ static void
> v9fs_post_lxattr_check(V9fsState *s,
> V9fsXattrState *vs,
> ssize_t err)
> {
> if (err< 0) {
> - err = -errno;
> free_fid(s, vs->xattr_fidp->fid);
> goto out;
> }
> @@ -3548,7 +3475,6 @@ static void
> v9fs_readlink_post_readlink(V9fsState *s, V9fsReadLinkState *vs,
> int err)
> {
> if (err< 0) {
> - err = -errno;
> goto out;
> }
> vs->offset += pdu_marshal(vs->pdu, vs->offset,
> "s",&vs->target);
>
>
>
[-- Attachment #2: Type: text/html, Size: 84750 bytes --]
next prev parent reply other threads:[~2011-05-14 0:00 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-05-08 12:36 [Qemu-devel] [PATCH] Clean up virtio-9p error handling code Sassan Panahinejad
2011-05-08 18:34 ` Venkateswararao Jujjuri
2011-05-09 9:21 ` Sassan Panahinejad
2011-05-13 23:59 ` Venkateswararao Jujjuri [this message]
2011-05-15 14:59 ` Sassan Panahinejad
2011-05-15 16:43 ` Sassan Panahinejad
2011-05-17 19:52 ` Venkateswararao Jujjuri
2011-05-18 7:37 ` Sassan Panahinejad
2011-06-08 16:21 ` Sassan Panahinejad
2011-06-28 12:25 ` Sassan Panahinejad
2011-06-28 14:22 ` Venkateswararao Jujjuri
2011-06-28 17:00 ` Sassan Panahinejad
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4DCDC5F5.9090508@linux.vnet.ibm.com \
--to=jvrao@linux.vnet.ibm.com \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).