Re: [Xen-devel] [PATCH] tools/libxc: Fix domid parameter types

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 09/10/17 16:19, Wei Liu wrote:
> On Mon, Oct 09, 2017 at 03:51:49PM +0100, Andrew Cooper wrote:
>> On 09/10/17 15:47, Wei Liu wrote:
>>> On Fri, Oct 06, 2017 at 08:00:00PM +0100, Andrew Cooper wrote:
>>>> Mixed throughout libxc are uint32_t, int, and domid_t for domid parameters.
>>>> With a signed type, and an explicitly 16-bit type, it is exceedingly 
>>>> difficult
>>>> to construct an INVALID_DOMID constant which works with all of them.  (The
>>>> main problem being that domid_t gets unconditionally zero extended when
>>>> promoted to int for arithmatic.)
>>>>
>>>> Libxl uses uint32_t consistently everywhere, so alter libxc to match.
>>> I would rather using domid_t throughout in libxc. Is there any problem
>>> with that?
>> That would cause implicit truncation between libxl's idea of a domid,
>> and libxc's idea of a domid.  In practice, it means any libxl domid with
>> the upper 16 bits set may start to work (on the wrong domain!) where
>> they may have failed previously.
>>
> But that's already the case for a lot of hypercalls -- domid_t is widely
> used in public headers. The truncation happens regardless of what libxc
> uses.

True.

>
>> Finally, it won't fix the INVALID_DOMID constant problem, as xc_dom.h
>> leaks fully into libxl.
>>
> Sorry, I don't follow. What do you want to do?

What this patch does, and move everything to using uint32_t.

>
> Surely INVALID_DOMID should be part of public ABI?

There is no need for any users of the libxl, libxc or Xen public API to
pass an invalid domid.

However, there is a need for those libraries internally to have the
notion of an invalid domid for initialisation/checking purposes.

>  How does something in
> xc_dom.h leaking (what is leaking?) change that?

Currently, libxl_internal.h has

#define INVALID_DOMID ~0

As part of the gnt/dombuidler series, I needed to add INVALID_DOMID to
xc_dom.h so I can initialise dom->{console,xenstore}_domid with
something which doesn't alias 0 (which is a valid domid), and use it
later to confirm that the caller has passed appropriate values.

libxl_internal.h includes xc_dom.h, so the define has to move rather
than being duplicated.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel