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@lists.xen.org https://lists.xen.org/xen-devel