On 18.06.2021 13:34, Andrew Cooper wrote:
> On 18/06/2021 00:39, Daniel P. Smith wrote:
>> @@ -197,16 +204,21 @@ bool __init has_xsm_magic(paddr_t start)
>>
>> int __init register_xsm(struct xsm_operations *ops)
>> {
>> - if ( verify(ops) )
>> + if ( xsm_ops_registered != XSM_OPS_UNREGISTERED )
>> + return -EAGAIN;
>
> I know you moved this around the function, but it really isn't -EAGAIN
> material any more. It's "too late - nope".
>
> -EEXIST is probably best for "I'm never going to tolerate another call".
>
>> +
>> + if ( !ops )
>> {
>> - printk(XENLOG_ERR "Could not verify xsm_operations structure\n");
>> + xsm_ops_registered = XSM_OPS_REG_FAILED;
>> + printk(XENLOG_ERR "Invalid xsm_operations structure registered\n");
>> return -EINVAL;
>
> Honestly, I'd be half tempted to declare register_xsm() with
> __nonnull(0) and let the compiler reject any attempt to pass a NULL ops
> pointer.
>
> Both callers pass a pointer to a static singleton objects.
Why check at all when the source of the arguments is all internal?
We don't check pointers to be non-NULL elsewhere, with a few odd
exceptions (which imo should all be dropped).
Jan
