Re: [PATCH 2/5] libxencall: osdep_hypercall() should return long

From: Jan Beulich <jbeulich@suse.com>
To: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: "Wei Liu" <wl@xen.org>, "Roger Pau Monné" <roger.pau@citrix.com>,
	"Ian Jackson" <iwj@xenproject.org>,
	"Juergen Gross" <jgross@suse.com>,
	"xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>
Subject: Re: [PATCH 2/5] libxencall: osdep_hypercall() should return long
Date: Fri, 18 Jun 2021 15:42:25 +0200	[thread overview]
Message-ID: <31a3f6e7-2fe7-e25b-8e15-82940c9ef03f@suse.com> (raw)
In-Reply-To: <a8b032ec-bafa-ef50-825f-26e975e64c69@citrix.com>

On 18.06.2021 15:26, Andrew Cooper wrote:
> On 18/06/2021 11:23, Jan Beulich wrote:
>> Some hypercalls, memory-op in particular, can return values requiring
>> more than 31 bits to represent. Hence the underlying layers need to make
>> sure they won't truncate such values. (Note that for Solaris the
>> function also gets renamed, to match the other OSes.)
> 
> Spot the environment which obviously hasn't been compiled since the
> 4.5(?) timeframe...
> 
> Also, I'm fairly sure the comment in the NetBSD version is false when it
> says it is copying the Linux way of doing things - I'm pretty sure it
> means copying the FreeBSD way.

It doesn't say "copy", but "mimic", and I think what it means is the
effect for its callers. Therefore I think the comment makes sense in
its current shape. And I don't think I should change it right here
anyway.

>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> I think the commit message needs to state that this doesn't fix
> truncation in the Linux or Solaris, nor the truncation in the
> xencall{0..5}() public APIs.  It only fixes truncation issues for
> FreeBSD, MiniOS and NetBSD.

I've added

"Due to them merely propagating ioctl()'s return value, this change is
 benign on Linux and Solaris. IOW there's an actual effect here only for
 the BSDs and MiniOS, but even then further adjustments are needed at the
 xencall<N>() level."

> With a suitable adjustment, and ideally a fix to the NetBSD comment,
> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>

Thanks.

Jan