From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:52403) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QN5fU-0004vs-Dc for qemu-devel@nongnu.org; Thu, 19 May 2011 12:01:45 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QN5fT-0006li-BE for qemu-devel@nongnu.org; Thu, 19 May 2011 12:01:44 -0400 Received: from mail-gx0-f173.google.com ([209.85.161.173]:46210) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QN5fS-0006la-U0 for qemu-devel@nongnu.org; Thu, 19 May 2011 12:01:43 -0400 Received: by gxk26 with SMTP id 26so1123950gxk.4 for ; Thu, 19 May 2011 09:01:42 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <4DD53729.5000104@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> <4DD41301.5060804@linux.vnet.ibm.com> <4DD53729.5000104@linux.vnet.ibm.com> Date: Thu, 19 May 2011 17:01:42 +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 Cc: qemu-devel@nongnu.org On Thu, May 19, 2011 at 4:28 PM, Venkateswararao Jujjuri wrote: > On 05/18/2011 10:37 PM, Stefan Hajnoczi wrote: >> >> On Wed, May 18, 2011 at 7:42 PM, Venkateswararao Jujjuri >> =A0wrote: >>> >>> On 05/18/2011 02:43 AM, Stefan Hajnoczi wrote: >>>> >>>> On Tue, May 17, 2011 at 8:43 PM, Venkateswararao Jujjuri (JV) >>>> =A0 =A0wrote: >>>>> >>>>> 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-x= attr.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-nest= ed-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. >>>>> =A0See >>>>> + * 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? =A0You 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. >>> >>> tbuf is introduced for re-entrent purpose. >>> We should be calling v9fs_string_init() on this though. >> >> I see no issue here and no safety added by using a local variable. >> Can you explain what you're trying to do? > > No issues in the current usage. Just looking at the micro level of this > routine, > we have an passd-in buffer passed under lock and is getting updated outsi= de > the lock (worker thread). > Can take it out, not an issue. Please take it out since it isn't needed. Stefan