Re: [Xen-devel] [PATCH] libxc: add xc_domain_add_to_physmap_batch to wrap XENMEM_add_to_physmap_batch

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 16.06.17 at 11:36, <blackskygg@xxxxxxxxx> wrote:
> 2017-06-16 16:45 GMT+08:00 Jan Beulich <JBeulich@xxxxxxxx>:
>>>>> On 16.06.17 at 06:55, <blackskygg@xxxxxxxxx> wrote:
>>> --- a/tools/libxc/include/xenctrl.h
>>> +++ b/tools/libxc/include/xenctrl.h
>>> @@ -1372,6 +1372,15 @@ int xc_domain_add_to_physmap(xc_interface *xch,
>>>                               unsigned long idx,
>>>                               xen_pfn_t gpfn);
>>>
>>> +int xc_domain_add_to_physmap_batch(xc_interface *xch,
>>> +                                   uint32_t domid,
>>> +                                   uint32_t foreign_domid,
>>
>> I'm not exactly sure what the libxc coding rules are, but I'd expect
>> these both to be domid_t, ...
>>
> 
> I was planning to make them domid_t, but according to the other
> domid-parameters' types
> in the file, and they are all uint32_t, so I finally decided on uint32_t.

You'll want to see what the maintainers of the code say, but my
general position on things like this is to try to avoid copying prior
mistakes.

>>> +                                   unsigned int space,
>>> +                                   uint16_t size,
>>
>> ... this one to be unsigned int, ...
> 
> In the xen_add_to_physmap_batch struct, both @space and @size are
> uint16_t, so I think
> I should have made @space uint16_t, too. I'll fix this. Or do you have
> any good reasons to
> make both of them unsigned int?

Again, I'm not a maintainer of this code, but in the hypervisor we
view it the other way around: You need to have a good reason
to use fixed-width types.

Jan


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