From mboxrd@z Thu Jan  1 00:00:00 1970
From: Wei Liu <wei.liu2@citrix.com>
Subject: Re: [PATCH XEN v6 20/32] tools/libs/foreignmemory:
 Support err == NULL to map.
Date: Wed, 9 Dec 2015 16:16:17 +0000
Message-ID: <20151209161617.GP23818@citrix.com>
References: <1449141675.4424.125.camel@citrix.com>
	<1449141749-14940-1-git-send-email-ian.campbell@citrix.com>
	<1449141749-14940-21-git-send-email-ian.campbell@citrix.com>
	<20151209122219.GH23818@citrix.com>
	<1449664909.16124.204.camel@citrix.com>
	<20151209125638.GN23818@citrix.com>
	<1449666931.16124.234.camel@citrix.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="iso-8859-1"
Content-Transfer-Encoding: quoted-printable
Return-path: <xen-devel-bounces@lists.xen.org>
Content-Disposition: inline
In-Reply-To: <1449666931.16124.234.camel@citrix.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Ian Campbell <ian.campbell@citrix.com>
Cc: ian.jackson@eu.citrix.com, Wei Liu <wei.liu2@citrix.com>, xen-devel@lists.xen.org
List-Id: xen-devel@lists.xenproject.org

On Wed, Dec 09, 2015 at 01:15:31PM +0000, Ian Campbell wrote:
> On Wed, 2015-12-09 at 12:56 +0000, Wei Liu wrote:
> > On Wed, Dec 09, 2015 at 12:41:49PM +0000, Ian Campbell wrote:
> > > On Wed, 2015-12-09 at 12:22 +0000, Wei Liu wrote:
> > > > > diff --git a/tools/libs/foreignmemory/core.c
> > > > > b/tools/libs/foreignmemory/core.c
> > > > > index 21dc7ee..91bea55 100644
> > > > > --- a/tools/libs/foreignmemory/core.c
> > > > > +++ b/tools/libs/foreignmemory/core.c
> > > > > @@ -14,6 +14,8 @@
> > > > > =A0 */
> > > > > =A0
> > > > > =A0#include <stdlib.h>
> > > > > +#include <assert.h>
> > > > > +#include <errno.h>
> > > > > =A0
> > > > > =A0#include "private.h"
> > > > > =A0
> > > > > @@ -64,7 +66,33 @@ void
> > > > > *xenforeignmemory_map(xenforeignmemory_handle
> > > > > *fmem,
> > > > > =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=
=A0=A0=A0=A0=A0=A0=A0uint32_t dom, int prot,
> > > > > =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=
=A0=A0=A0=A0=A0=A0=A0const xen_pfn_t *arr, int *err, size_t
> > > > > num)
> > > > > =A0{
> > > > > -=A0=A0=A0=A0return osdep_xenforeignmemory_map(fmem, dom, prot, a=
rr, err,
> > > > > num);
> > > > > +=A0=A0=A0=A0void *ret;
> > > > > +=A0=A0=A0=A0int *err_to_free =3D NULL;
> > > > > +
> > > > > +=A0=A0=A0=A0if ( err =3D=3D NULL )
> > > > > +=A0=A0=A0=A0=A0=A0=A0=A0err =3D err_to_free =3D malloc(num * siz=
eof(int));
> > > > > +
> > > > =

> > > > malloc can fail, which means when user passes in err=3D=3DNULL ther=
e is
> > > > two
> > > > possible behaviours of this function?
> > > =

> > > Partially/Full success and "fail entirely reporting an error" you mea=
n?
> > > There are always those two possibilities, yes, regardless of err=3D=
=3DNULL.
> > > I
> > > think that's inline with the doc comment too.
> > > =

> > > If that's not what you were asking then I'm afraid I don't see what y=
ou
> > > are
> > > getting at.
> > > =

> > =

> > What I meant was we should check err !=3D NULL after malloc because
> > osdep_xenforeignmeory_map dereferences it unconditionally.
> =

> Ah, I appear to have managed to put that check in
> "tools/libs/foreignmemory: optimise map(num=3D=3D1, err=3D=3DNULL) case" =
by
> mistake, so I was confused because I was looking at the final version of
> the file.
> =

> I shall move it, thanks.
> =


With that fixed:

Acked-by: Wei Liu <wei.liu2@citrix.com>

Thanks

> Ian.