Re: [Xen-devel] [PATCH v3 5/7] x86/livepatch: Fail the build if duplicate symbols exist

[Jan Beulich <jbeulich@xxxxxxxx>]

On 29.10.2019 18:06, Andrew Cooper wrote:
> On 24/10/2019 13:03, Jan Beulich wrote:
>> On 23.10.2019 15:58, Andrew Cooper wrote:
>>> --- a/xen/common/Kconfig
>>> +++ b/xen/common/Kconfig
>>> @@ -361,9 +361,23 @@ config FAST_SYMBOL_LOOKUP
>>>  
>>>       If unsure, say Y.
>>>  
>>> +config ENFORCE_UNIQUE_SYMBOLS
>>> +   bool "Enforce unique symbols" if LIVEPATCH
>>> +   default y if LIVEPATCH
>> Instead of two identical "if", why not "depends on LIVEPATCH"?
>>
>>> +   ---help---
>>> +     Multiple symbols with the same name aren't generally a problem
>>> +     unless Live patching is to be used.
>>> +
>>> +     Livepatch loading involves resolving relocations against symbol
>>> +     names, and attempting to a duplicate symbol in a livepatch will
>>> +     result in incorrect livepatch application.
>>> +
>>> +     This option should be used to ensure that a build of Xen can have a
>>> +     livepatch build and apply correctly.
>>> +
>>>  config SUPPRESS_DUPLICATE_SYMBOL_WARNINGS
>>> -   bool "Suppress duplicate symbol warnings" if !LIVEPATCH
>>> -   default y if !LIVEPATCH
>>> +   bool "Suppress duplicate symbol warnings" if !ENFORCE_UNIQUE_SYMBOLS
>>> +   default y if !ENFORCE_UNIQUE_SYMBOLS
>> Similarly here then. With this changed, or with a proper reason
>> supplied
>> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
> 
> That's a question for the author of c/s 064a2652233 to answer...  I'm
> merely following the prevailing style.

"Prevailing style" seems a little bold considering that according to
my grep-ing there's exactly on other such instance (VGA). I.e. you'd
grow the "badness" by 50% when you could equally well shrink it by
this same percentage.

Jan

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