Re: [RFC 6/6] acpi:arm64: Add support for parsing IORT table

[Julien Grall <julien.grall@arm.com>]

Hi Sameer,

On 21/09/17 01:37, Goel, Sameer wrote:
>>>>
>>>> [...]
>>>>
>>>>> diff --git a/xen/include/xen/lib.h b/xen/include/xen/lib.h
>>>>> index 995a85a..3785fae 100644
>>>>> --- a/xen/include/xen/lib.h
>>>>> +++ b/xen/include/xen/lib.h
>>>>> @@ -9,7 +9,12 @@
>>>>>    #include <asm/bug.h>
>>>>>
>>>>>    #define BUG_ON(p)  do { if (unlikely(p)) BUG();  } while (0)
>>>>> -#define WARN_ON(p) do { if (unlikely(p)) WARN(); } while (0)
>>>>> +#define WARN_ON(p) ({                                      \
>>>>> +    int __ret_warn_on = !!(p);                             \
>>>>> +    if (unlikely(__ret_warn_on))                           \
>>>>> +        WARN();                                            \
>>>>> +    unlikely(__ret_warn_on);                               \
>>>>> +})
>>>>
>>>> hmmmm. Why this change?
>>> Was getting a compilation error when I was using WARN_ON as a conditional
>>> in an if statement regarding the return value. So removed the loop. This
>>> looks similar to Linux now.
>>
>> This should belong to a separate patch with a commit message explaining why the change.
> 
> Agreed. Since this is more of a Linux compat function, I do not want to change the system-wide behavior. I will move this in the IORT code.

I don't see any reason to keep this implementation only for IORT. It 
would be possible to have similar construction in Xen.

So I would first try to see if maintainers would be willing to accept a 
system-wide change before thinking to re-implement WARN_ON in IORT.

Cheers,

-- 
Julien Grall

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