From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:60326) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QAfXl-0005yY-N6 for qemu-devel@nongnu.org; Fri, 15 Apr 2011 05:42:26 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QAfXk-0004nz-A5 for qemu-devel@nongnu.org; Fri, 15 Apr 2011 05:42:25 -0400 Received: from mail-gy0-f173.google.com ([209.85.160.173]:61043) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QAfXk-0004nr-6A for qemu-devel@nongnu.org; Fri, 15 Apr 2011 05:42:24 -0400 Received: by gyg4 with SMTP id 4so980404gyg.4 for ; Fri, 15 Apr 2011 02:42:23 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <1302858357-11309-1-git-send-email-mohan@in.ibm.com> References: <1302858357-11309-1-git-send-email-mohan@in.ibm.com> Date: Fri, 15 Apr 2011 10:42:21 +0100 Message-ID: From: Stefan Hajnoczi Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH] virtio-9p: Make rpath function thread safe List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "M. Mohan Kumar" Cc: qemu-devel@nongnu.org On Fri, Apr 15, 2011 at 10:05 AM, M. Mohan Kumar wrote: > Use dynamically allocated memory to return concatenated path > (fs_root and file path) instead of a static buffer. Caller has to free > the memory. > > Signed-off-by: M. Mohan Kumar > --- > This patch depends on my chroot patchset. > http://patchwork.ozlabs.org/patch/89033/ > > =A0hw/9pfs/virtio-9p-local.c | =A0551 ++++++++++++++++++++++++++---------= ---------- > =A0hw/file-op-9p.h =A0 =A0 =A0 =A0 =A0 | =A0 10 +- > =A02 files changed, 322 insertions(+), 239 deletions(-) Would it be possible to build a const char *fs_ctx->path once in the PDU handler for each operation that takes a path instead of pushing so many rpath() calls down into virtio-9p-local.c? That way you don't need to worry about constructing and freeing paths over and over. I think it's time to kill errno once and for all in virtio-9p. The errno global variable usage is getting out of hand; pretty much every patch is adding boilerplate code to juggle around errno and avoid trampling it. This new qemu_free() call means we need to juggle in new places. Please stop adding more errno juggling and just use -errno return values. That change needs to happen before this patch. > @@ -137,29 +164,10 @@ static int local_lstat(FsContext *fs_ctx, const cha= r *path, struct stat *stbuf) > =A0 =A0 =A0 =A0 } > =A0 =A0 =A0 =A0 if (getxattr(rpath(fs_ctx, path), "user.virtfs.rdev", &tm= p_dev, > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 sizeof(dev_t)) > 0) { Leak. > =A0 =A0 =A0 =A0 tsize =3D readlink(rpath(fs_ctx, path), buf, bufsz); Leak. > -static void local_rewinddir(FsContext *ctx, DIR *dir) > +static void local_rewinddir(FsContext *fs_ctx, DIR *dir) > =A0{ > =A0 =A0 return rewinddir(dir); > =A0} Please do this in a separate patch. > =A0 =A0 =A0 =A0 err =3D local_set_xattr(rpath(fs_ctx, newpath), credp); Leak. Stefan