Re: [PATCH 5/6] xsm: expanding function related macros in dummy.h

["Daniel P. Smith" <dpsmith@xxxxxxxxxxxxxxxxxxxx>]

On 6/18/21 8:44 AM, Jan Beulich wrote:
> On 18.06.2021 14:40, Jan Beulich wrote:
>> On 18.06.2021 14:03, Andrew Cooper wrote:
>>> On 18/06/2021 00:39, Daniel P. Smith wrote:
>>>> diff --git a/xen/xsm/dummy.h b/xen/xsm/dummy.h
>>>> index 7e2bb09dac..0f8ea163af 100644
>>>> --- a/xen/xsm/dummy.h
>>>> +++ b/xen/xsm/dummy.h
>>>> @@ -9,7 +9,7 @@
>>>>   *
>>>>   *
>>>>   *  Each XSM hook implementing an access check should have its first 
>>>> parameter
>>>> - *  preceded by XSM_DEFAULT_ARG (or use XSM_DEFAULT_VOID if it has no
>>>> + *  preceded by (or use XSM_DEFAULT_VOID if it has no
>>>>   *  arguments). The first non-declaration statement shold be 
>>>> XSM_ASSERT_ACTION
>>>>   *  with the expected type of the hook, which will either define or check 
>>>> the
>>>>   *  value of action.
>>>> @@ -47,14 +47,12 @@ void __xsm_action_mismatch_detected(void);
>>>>   * xsm_default_t argument available, so the value from the assertion is 
>>>> used to
>>>>   * initialize the variable.
>>>>   */
>>>> -#define XSM_INLINE __maybe_unused
>>>
>>> Nothing in a header file should ever need __maybe_unused.  Now that the
>>> !XSM case has been untangled, I think this can be dropped, rather than
>>> expanded inline.
>>>
>>>> -
>>>> -#define XSM_DEFAULT_ARG /* */
>>>>  #define XSM_DEFAULT_VOID void
>>>
>>> XSM_DEFAULT_VOID needs to disappear too.  I can't see what it is even
>>> doing before the cleanup, because if it is missing, you'll fail the
>>> compile for using K&R style functions.
>>
>> You need to look at the state before patch 3 to see its purpose. Patch 3
>> removed the other variant, and hence the need for this one as well, but
>> I think it is reasonable to not clean up everything in one go (unless
>> it would mean touching exactly the same code a 2nd time later on).
> 
> Albeit, having looked at the patch itself, I agree it should be dropped
> here together with XSM_DEFAULT_ARG, of which it is (was) a companion.
> But again, all provided there is agreement to remove the top level XSM
> option, which I personally don't think is a good idea.

ack, will be removing it

v/r,
dps