Re: [Xen-devel] [PATCH v3 3/3] x86/string: Clean up x86/string.h

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 31.05.17 at 14:42, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 15/05/17 15:22, Jan Beulich wrote:
>>>>> On 15.05.17 at 15:08, <andrew.cooper3@xxxxxxxxxx> wrote:
>>> On 15/05/17 11:19, Jan Beulich wrote:
>>>>  >>> On 15.05.17 at 12:08, <JBeulich@xxxxxxxx> wrote:
>>>>>>>> On 12.05.17 at 19:35, <andrew.cooper3@xxxxxxxxxx> wrote:
>>>>>> --- a/xen/include/asm-x86/string.h
>>>>>> +++ b/xen/include/asm-x86/string.h
>>>>>> @@ -2,13 +2,23 @@
>>>>>>  #define __X86_STRING_H__
>>>>>>  
>>>>>>  #define __HAVE_ARCH_MEMCPY
>>>>>> -#define memcpy(t,f,n) (__builtin_memcpy((t),(f),(n)))
>>>>>> +void *memcpy(void *dest, const void *src, size_t n);
>>>>>> +#define memcpy(d, s, n) __builtin_memcpy(d, s, n)
>>>>>>  
>>>>>> -/* Some versions of gcc don't have this builtin. It's non-critical 
>>>>>> anyway. 
>>>>> */
>>>>>>  #define __HAVE_ARCH_MEMMOVE
>>>>>> -extern void *memmove(void *dest, const void *src, size_t n);
>>>>>> +void *memmove(void *dest, const void *src, size_t n);
>>>>>> +#define memmove(d, s, n) __builtin_memmove(d, s, n)
>>>>>>  
>>>>>>  #define __HAVE_ARCH_MEMSET
>>>>>> -#define memset(s,c,n) (__builtin_memset((s),(c),(n)))
>>>>>> +void *memset(void *dest, int c, size_t n);
>>>>>> +#define memset(s, c, n) __builtin_memset(s, c, n)
>>>>> Now that xen/string.h has the exact same declarations and
>>>>> definitions already, why don't you simply delete the overrides
>>>>> from here?
>>>> Hmm, wait - I guess you need to keep them because of the custom
>>>> implementation. That's awkward, there shouldn't be a need to have
>>>> redundant declarations just because there are custom
>>>> implementations. How about making __HAVE_ARCH_* serve both
>>>> purposes, by allowing it to have different values (besides being
>>>> defined or undefined)?
>>> I don't understand how you would intend this new __HAVE_ARCH_* to work.
>> E.g. __HAVE_ARCH_* = 2 meaning arch provides declaration and
>> definition (i.e. generic header and source skip theirs), while
>> __HAVE_ARCH_* = 1 meaning arch provides just a definition, but
>> the generic declaration and macro (where applicable) are fine (i.e.
>> only the generic source skips its piece of code).
> 
> I really don't thing its worth effort to make this change and diverge
> from the Linux meaning.
> 
> It is not like these declarations are going to change often.

Well, okay then. Feel free to add my ack.

Jan


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