Re: [PATCH v3 1/5] common/vmap: Fall back to simple allocator when !HAS_VMAP

[Jan Beulich <jbeulich@xxxxxxxx>]

On 29.11.2024 12:14, Luca Fancellu wrote:
> Hi Jan,
> 
>> On 29 Nov 2024, at 11:06, Jan Beulich <jbeulich@xxxxxxxx> wrote:
>>
>> On 29.11.2024 10:12, Luca Fancellu wrote:
>>> --- a/xen/include/xen/xvmalloc.h
>>> +++ b/xen/include/xen/xvmalloc.h
>>> @@ -40,20 +40,46 @@
>>>     ((typeof(ptr))_xvrealloc(ptr, offsetof(typeof(*(ptr)), field[nr]), \
>>>                              __alignof__(typeof(*(ptr)))))
>>>
>>> +#ifdef CONFIG_HAS_VMAP
>>> +
>>> /* Free any of the above. */
>>> void xvfree(void *va);
>>>
>>> +/* Underlying functions */
>>> +void *_xvmalloc(size_t size, unsigned int align);
>>> +void *_xvzalloc(size_t size, unsigned int align);
>>> +void *_xvrealloc(void *va, size_t size, unsigned int align);
>>> +
>>> +#else /* !CONFIG_HAS_VMAP */
>>> +
>>> +static inline void xvfree(void *va)
>>> +{
>>> +    xfree(va);
>>> +}
>>> +
>>> +static inline void *_xvmalloc(size_t size, unsigned int align)
>>> +{
>>> +    return _xmalloc(size, align);
>>> +}
>>> +
>>> +static inline void *_xvzalloc(size_t size, unsigned int align)
>>> +{
>>> +    return _xzalloc(size, align);
>>> +}
>>> +
>>> +static inline void *_xvrealloc(void *va, size_t size, unsigned int align)
>>> +{
>>> +    return _xrealloc(va, size, align);
>>> +}
>>
>> Just to double check: Was it at least considered to use simple #define-s
>> to effect the aliasing? Wrapper functions like the above ones have the
>> downside of needing touching (easy to miss) when the wrapped function
>> types change in whichever minor way. (And yes, I do understand that we
>> generally aim at using inline functions in preference to macros.)
> 
> Yes, I think I tried and I didn’t have issues using #define-s, I asked here
> https://patchwork.kernel.org/project/xen-devel/patch/20241115105036.218418-2-luca.fancellu@xxxxxxx/#26123448
> about a preferred approach, but I didn’t have any reply, so I went for what
> I believed was preferred as you said, static inline-s instead of macros.

As Andrew's idea didn't work out, personally I think the simple #define
approach you suggested would be preferable in this case. There is in
particular no type-safety concern here, as the wrapped functions will
all have the intended type checking applied.

Jan