From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from [140.186.70.92] (port=58254 helo=eggs.gnu.org)
	by lists.gnu.org with esmtp (Exim 4.43) id 1Pyo5X-0006P5-Nl
	for qemu-devel@nongnu.org; Sun, 13 Mar 2011 12:24:17 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <stefanha@gmail.com>) id 1Pyo5W-0003Rc-IQ
	for qemu-devel@nongnu.org; Sun, 13 Mar 2011 12:24:15 -0400
Received: from mail-yw0-f45.google.com ([209.85.213.45]:37961)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <stefanha@gmail.com>) id 1Pyo5W-0003RY-DM
	for qemu-devel@nongnu.org; Sun, 13 Mar 2011 12:24:14 -0400
Received: by ywl41 with SMTP id 41so2187898ywl.4
	for <qemu-devel@nongnu.org>; Sun, 13 Mar 2011 09:24:13 -0700 (PDT)
MIME-Version: 1.0
In-Reply-To: <1299347533-17047-4-git-send-email-aneesh.kumar@linux.vnet.ibm.com>
References: <1299347533-17047-1-git-send-email-aneesh.kumar@linux.vnet.ibm.com>
	<1299347533-17047-4-git-send-email-aneesh.kumar@linux.vnet.ibm.com>
Date: Sun, 13 Mar 2011 16:24:13 +0000
Message-ID: <AANLkTikxwC6yPmQj2QSkRjVRcSB3EwZ72a9A6_GP_-t6@mail.gmail.com>
Subject: Re: [Qemu-devel] [PATCH -V3 4/8] hw/9pfs: Implement syncfs
From: Stefan Hajnoczi <stefanha@gmail.com>
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: quoted-printable
List-Id: qemu-devel.nongnu.org
List-Unsubscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
Cc: aliguori@us.ibm.com, "Venkateswararao Jujjuri (JV)" <jvrao@linux.vnet.ibm.com>, qemu-devel@nongnu.org

On Sat, Mar 5, 2011 at 5:52 PM, Aneesh Kumar K.V
<aneesh.kumar@linux.vnet.ibm.com> wrote:
> +static void v9fs_post_do_syncfs(V9fsState *s, V9fsPDU *pdu, int err)
> +{
> + =A0 =A0if (err =3D=3D -1) {
> + =A0 =A0 =A0 =A0err =3D -errno;
> + =A0 =A0}

errno may have been clobbered by any standard library function/syscall
invoked by this thread before v9fs_post_do_syncfs() was called.  Why
not pass the -errno in the function argument?

static void v9fs_post_do_syncfs(V9fsState *s, V9fsPDU *pdu, int err)
{
 =A0 =A0complete_pdu(s, pdu, err);
}

I strongly suggest doing a separate patch series to clean up of all of
virtio-9p to stop using errno.  It's bad practice and I've caught
several errno mistakes in the virtio-9p patches I've reviewed - I bet
there are other instances lurking around.

> +static void v9fs_syncfs(V9fsState *s, V9fsPDU *pdu)
> +{
> + =A0 =A0int err;
> + =A0 =A0int32_t fid;
> + =A0 =A0size_t offset =3D 7;
> + =A0 =A0V9fsFidState *fidp;
> +
> + =A0 =A0pdu_unmarshal(pdu, offset, "d", &fid);
> + =A0 =A0fidp =3D lookup_fid(s, fid);
> + =A0 =A0if (fidp =3D=3D NULL) {
> + =A0 =A0 =A0 =A0err =3D -ENOENT;
> + =A0 =A0 =A0 =A0v9fs_post_do_syncfs(s, pdu, err);
> + =A0 =A0 =A0 =A0return;
> + =A0 =A0}

This wasn't setting errno but passed the error in err.  It can stay
like this if you change v9fs_post_do_syncfs().

Stefan