From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:57669) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QMdI4-0008IP-38 for qemu-devel@nongnu.org; Wed, 18 May 2011 05:43:40 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QMdI3-0004Uq-0T for qemu-devel@nongnu.org; Wed, 18 May 2011 05:43:40 -0400 Received: from mail-yi0-f45.google.com ([209.85.218.45]:33344) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QMdI2-0004Um-RM for qemu-devel@nongnu.org; Wed, 18 May 2011 05:43:38 -0400 Received: by yib19 with SMTP id 19so542701yib.4 for ; Wed, 18 May 2011 02:43:38 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <1305661431-21289-7-git-send-email-jvrao@linux.vnet.ibm.com> References: <1305661431-21289-1-git-send-email-jvrao@linux.vnet.ibm.com> <1305661431-21289-7-git-send-email-jvrao@linux.vnet.ibm.com> Date: Wed, 18 May 2011 10:43:38 +0100 Message-ID: From: Stefan Hajnoczi Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [V2 06/25] [virtio-9p] coroutines for readlink List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Venkateswararao Jujjuri (JV)" Cc: aliguori@us.ibm.com, qemu-devel@nongnu.org, stefanha@linux.vnet.ibm.com On Tue, May 17, 2011 at 8:43 PM, Venkateswararao Jujjuri (JV) wrote: > Signed-off-by: Venkateswararao Jujjuri " > --- > =A0Makefile.objs =A0 =A0 =A0 =A0 =A0 =A0| =A0 =A02 +- > =A0hw/9pfs/cofs.c =A0 =A0 =A0 =A0 =A0 | =A0 42 ++++++++++++++++++++++++++= ++++++++++++++++ > =A0hw/9pfs/virtio-9p-coth.h | =A0 =A01 + > =A0hw/9pfs/virtio-9p.c =A0 =A0 =A0| =A0 27 ++++----------------------- > =A0hw/9pfs/virtio-9p.h =A0 =A0 =A0| =A0 =A03 ++- > =A05 files changed, 50 insertions(+), 25 deletions(-) > =A0create mode 100644 hw/9pfs/cofs.c > > diff --git a/Makefile.objs b/Makefile.objs > index 96f6a24..36005bb 100644 > --- a/Makefile.objs > +++ b/Makefile.objs > @@ -297,7 +297,7 @@ hw-obj-$(CONFIG_SOUND) +=3D $(sound-obj-y) > =A09pfs-nested-$(CONFIG_VIRTFS) =3D virtio-9p.o virtio-9p-debug.o > =A09pfs-nested-$(CONFIG_VIRTFS) +=3D =A0virtio-9p-local.o virtio-9p-xattr= .o > =A09pfs-nested-$(CONFIG_VIRTFS) +=3D =A0 virtio-9p-xattr-user.o virtio-9p= -posix-acl.o > -9pfs-nested-$(CONFIG_VIRTFS) +=3D virtio-9p-coth.o > +9pfs-nested-$(CONFIG_VIRTFS) +=3D virtio-9p-coth.o cofs.o > > =A0hw-obj-$(CONFIG_REALLY_VIRTFS) +=3D $(addprefix 9pfs/, $(9pfs-nested-y= )) > =A0$(addprefix 9pfs/, $(9pfs-nested-y)): QEMU_CFLAGS+=3D$(GLIB_CFLAGS) > diff --git a/hw/9pfs/cofs.c b/hw/9pfs/cofs.c > new file mode 100644 > index 0000000..6d94673 > --- /dev/null > +++ b/hw/9pfs/cofs.c > @@ -0,0 +1,42 @@ > + > +/* > + * Virtio 9p backend > + * > + * Copyright IBM, Corp. 2011 > + * > + * Authors: > + * =A0Aneesh Kumar K.V > + * > + * This work is licensed under the terms of the GNU GPL, version 2. =A0S= ee > + * the COPYING file in the top-level directory. > + * > + */ > + > +#include "fsdev/qemu-fsdev.h" > +#include "qemu-thread.h" > +#include "qemu-coroutine.h" > +#include "virtio-9p-coth.h" > + > +int v9fs_co_readlink(V9fsState *s, V9fsString *path, V9fsString *buf) > +{ > + =A0 =A0int err; > + =A0 =A0ssize_t len; > + =A0 =A0V9fsString tbuf; > + > + =A0 =A0tbuf.data =3D qemu_malloc(PATH_MAX); Why introduce tbuf when the buf is available? You end up having to copy back fields at the end of the function and load from an uninitialized address (tbuf.size) in the error case. > @@ -299,7 +282,7 @@ static void v9fs_string_init(V9fsString *str) > =A0 =A0 str->size =3D 0; > =A0} > > -static void v9fs_string_free(V9fsString *str) > +void v9fs_string_free(V9fsString *str) > =A0{ > =A0 =A0 qemu_free(str->data); > =A0 =A0 str->data =3D NULL; > @@ -421,7 +404,7 @@ v9fs_string_sprintf(V9fsString *str, const char *fmt,= ...) > =A0 =A0 str->size =3D err; > =A0} > > -static void v9fs_string_copy(V9fsString *lhs, V9fsString *rhs) > +void v9fs_string_copy(V9fsString *lhs, V9fsString *rhs) > =A0{ > =A0 =A0 v9fs_string_free(lhs); > =A0 =A0 v9fs_string_sprintf(lhs, "%s", rhs->data); Spurious changes? I don't see any users here. > @@ -1057,8 +1040,7 @@ static int stat_to_v9stat(V9fsState *s, V9fsString = *name, > =A0 =A0 v9fs_string_null(&v9stat->extension); > > =A0 =A0 if (v9stat->mode & P9_STAT_MODE_SYMLINK) { > - =A0 =A0 =A0 =A0ssize_t symlink_len; > - =A0 =A0 =A0 =A0err =3D v9fs_do_readlink(s, name, &v9stat->extension, &s= ymlink_len); > + =A0 =A0 =A0 =A0err =3D v9fs_co_readlink(s, name, &v9stat->extension); > =A0 =A0 =A0 =A0 if (err < 0) { > =A0 =A0 =A0 =A0 =A0 =A0 return err; > =A0 =A0 =A0 =A0 } > @@ -3591,7 +3573,6 @@ static void v9fs_readlink(void *opaque) > =A0 =A0 int32_t fid; > =A0 =A0 int err =3D 0; > =A0 =A0 V9fsFidState *fidp; > - =A0 =A0ssize_t symlink_len; > > =A0 =A0 pdu_unmarshal(pdu, offset, "d", &fid); > =A0 =A0 fidp =3D lookup_fid(pdu->s, fid); > @@ -3601,7 +3582,7 @@ static void v9fs_readlink(void *opaque) > =A0 =A0 } > > =A0 =A0 v9fs_string_init(&target); > - =A0 =A0err =3D v9fs_do_readlink(pdu->s, &fidp->path, &target, &symlink_= len); > + =A0 =A0err =3D v9fs_co_readlink(pdu->s, &fidp->path, &target); > =A0 =A0 if (err < 0) { > =A0 =A0 =A0 =A0 goto out; > =A0 =A0 } Ah, the unused length argument from patch 5 has been removed. It would be nice to do fixes earlier rather than later in the patch series, otherwise time is spent reviewing and commenting on code which gets removed. Stefan