[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v4 08/16] xen: Add limited support of VMware's hyper-call rpc



On 09/12/14 10:27, Jan Beulich wrote:
>>>> On 12.09.14 at 15:37, <boris.ostrovsky@xxxxxxxxxx> wrote:
>> On 09/11/2014 02:36 PM, Don Slutz wrote:
>>> +static inline uint16_t get_low_bits(uint32_t bits)
>>> +{
>>> +    return bits & 0xffff;
>>> +}
>>> +
>>> +static inline uint16_t get_high_bits(uint32_t bits)
>>> +{
>>> +    return bits >> 16;
>>> +}
>>> +
>>> +static inline uint32_t set_high_bits(uint32_t b, uint32_t val)
>>> +{
>>> +    return (val << 16) | get_low_bits(b);
>>> +}
> The names of all three functions (at least one of which I think I saw
> in a reply to an earlier patch - code duplication?) are bogus: How
> many high or low bits (and for the "high" case out of how many) are
> we talking about here?

I have no issue with changing the names.  How does

s/get_low_bits/get_low_16bits_of_32/
s/get_high_bits/get_high_16bits_of_32/
s/set_high_bits/set_high_16bits_of_32/

look as a fix?

    -Don Slutz


>>> +static inline void set_status(struct cpu_user_regs *ur, uint16_t val)
>>> +{
>>> +    /* VMware defines this to be only 32 bits */
>>> +    ur->rcx = (val << 16) | (ur->rcx & 0xffff);
>> Are you shifting val out of its size? Or will compiler automatically
>> promote this to uint32_t? (It does for me but I don't know if this is
>> something we can rely on).
> All operands more narrow than int get promoted to int.
>
> Jan
>

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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.