From: Christoph Egger <Christoph.Egger@amd.com>
Subject: Re: [PATCH] libxc: make unlock_page return error
Date: Thu, 19 Aug 2010 17:39:29 +0200
Message-ID: <201008191739.30032.Christoph.Egger@amd.com>
References: <201008181642.10467.Christoph.Egger@amd.com>
	<19565.17339.361079.70298@mariner.uk.xensource.com>
	<1282229841.3170.3073.camel@zakaz.uk.xensource.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
In-Reply-To: <1282229841.3170.3073.camel@zakaz.uk.xensource.com>
Content-Disposition: inline
Sender: xen-devel-bounces@lists.xensource.com
Errors-To: xen-devel-bounces@lists.xensource.com
To: Ian Campbell <Ian.Campbell@citrix.com>
Cc: "xen-devel@lists.xensource.com" <xen-devel@lists.xensource.com>, Ian Jackson <Ian.Jackson@eu.citrix.com>

On Thursday 19 August 2010 16:57:21 Ian Campbell wrote:
> On Thu, 2010-08-19 at 15:46 +0100, Ian Jackson wrote:
> > Christoph Egger writes ("[Xen-devel] [PATCH] libxc: make unlock_page 
return error"):
> > > As a result of debugging 'xend segfaults when starting',
> > > the attached patch makes unlock_pages return an error.
> >
> > Having read this in more detail, it seems that you're making
> > {lock,unlock}_pages return errno values on error.  That's fine but
> > it's quite unusual in libxc; all libxc functions normally return -1 on
> > error and set errno.
> >
> > So I think the unusual return value convention is worth a comment in
> > xc_private.h.  Can you please resubmit with such a comment ?
>
> I think it should just follow the normal libxc convention instead of
> commenting on the use of an unusual return value convention.
>
> I also think that such a patch must include (or be part of a series
> which) updates the callers to actually do something with the new return
> value, or else it is somewhat pointless.
>
> Given that this patch was only for debugging purposes for an issue which
> is now resolved is there any need?

The need I see for now is a future regression.

Christoph


-- 
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach b. Muenchen
Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632