Re: [XEN PATCH][for-4.19] domain: add ASSERT to help static analysis tools

[Jan Beulich <jbeulich@suse.com>]

On 03.11.2023 18:58, Nicola Vetrini wrote:
> Static analysis tools may detect a possible null
> pointer dereference at line 760 (the memcpy call)
> of xen/common/domain.c. This ASSERT helps them in
> detecting that such a condition is not possible
> and also provides a basic sanity check.

I disagree with this being a possible justification for adding such a
redundant assertion. More detail is needed on what is actually
(suspected to be) confusing the tool. Plus it also needs explaining
why (a) adding such an assertion helps and (b) how that's going to
cover release builds.

> --- a/xen/common/domain.c
> +++ b/xen/common/domain.c
> @@ -700,6 +700,8 @@ struct domain *domain_create(domid_t domid,
>  
>      if ( !is_idle_domain(d) )
>      {
> +        ASSERT(config);
> +
>          watchdog_domain_init(d);
>          init_status |= INIT_watchdog;

The assertion being redundant clearly requires it to be accompanied
by a comment. Otherwise it is going to be a prime subject of
redundancy elimination.

Jan