Re: [Xen-devel] [RFC PATCH for-next 04/18] x86/mem_sharing: cleanup code in various locations

[Jan Beulich <jbeulich@xxxxxxxx>]

On 26.09.2019 16:09, Tamas K Lengyel wrote:
> On Wed, Sep 25, 2019 at 10:15 AM Jan Beulich <jbeulich@xxxxxxxx> wrote:
>> On 25.09.2019 17:48, Tamas K Lengyel wrote:
>>> --- a/xen/arch/x86/hvm/hvm.c
>>> +++ b/xen/arch/x86/hvm/hvm.c
>>> @@ -1879,12 +1879,11 @@ int hvm_hap_nested_page_fault(paddr_t gpa, unsigned 
>>> long gla,
>>>      if ( npfec.write_access && (p2mt == p2m_ram_shared) )
>>>      {
>>>          ASSERT(p2m_is_hostp2m(p2m));
>>> -        sharing_enomem =
>>> -            (mem_sharing_unshare_page(currd, gfn, 0) < 0);
>>> +        sharing_enomem = mem_sharing_unshare_page(currd, gfn, 0);
>>
>> I guess the implication here is that the function can only return
>> -ENOMEM? Not very forward compatible, but well. However, if you
>> touch this already, shouldn't you at least make "sharing_enomem"
>> bool?
> 
> Correct, there is a BUG_ON for every other rc value but ENOMEM. We
> could turn it into a bool but I don't see a reason for it, perhaps
> there will be another rc value in the future that we want to handle
> gracefully.

At which point the variable's name will no longer be appropriate.
Hence my request to make it bool; at such a future point the code
would need touching again anyway if you (understandably) don't
want to make more than purely cosmetic changes now.

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel