[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


  • To: Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx>
  • From: Jan Beulich <jbeulich@xxxxxxxx>
  • Date: Wed, 8 Nov 2023 09:24:36 +0100
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=suse.com; dmarc=pass action=none header.from=suse.com; dkim=pass header.d=suse.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=4k2I7rMtePHodnnE/F0J8bnrAivnEquTzBtf+BHUxBY=; b=TGpZDmAefSSbqsv0DE8UG4uzFu5LkZIUg0D/Pt1c5k+NZ+A1O7APYRrW9LMy6xVhwGVSeLh9QwaOL7BFPwakzQ8lTLYgNiNI1zs6zbL82Po4HObu5HAhbdIg5WDu4oiyCDa1q6YNSzySozY/+ZS/DiQ0hK9Kt5sg4dgDTF40rnRoH3bvSvPZYYDQQWPI/+j5zAA/S2REdzm7YnLFGBj0OANR7+u1/bSclEmz+fiWEsS9g/T6565CPnXEmHoiiNa4At5Vl6kAhop1JTk1NvTSKjcOpd5okvdbtis1YCIYdZsJGRCTPWMZbgFKnoW0fr5DIHaSmDFgXv9N++ricUqjqw==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=WoUl6idtcr+CtA9UPZ0BOcvp2QxPdI+epSTHs9hbuHmybiHdo93/9j/hJdFU+k8cc7f+7FZQCBjI4cr8RGYFyx6/vu/rBHNuDfmQZqKGCexyHsNjmkSC25rMKQwjko8yVvHdEKIP+81WSR+HSpgjiyWExa/zGpguU77/1fyTH3XYTdaUZI0sSmLDoBXDfywJ20N8jBgeKfjJhwtmIndScLkuqzJNh7z1iJtnaCRcvKE7hpYaTPJHIUTouM7WSbGg3DiQrf304dQ3sztIoI1k8qj4jYq4g3Ml+qZ9nQhnguTrT5mleOA3HUh4QxynTTeqrajFlbfMplfZrQmbjY6euw==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=suse.com;
  • Cc: sstabellini@xxxxxxxxxx, michal.orzel@xxxxxxx, xenia.ragiadakou@xxxxxxx, ayan.kumar.halder@xxxxxxx, consulting@xxxxxxxxxxx, andrew.cooper3@xxxxxxxxxx, roger.pau@xxxxxxxxxx, George Dunlap <george.dunlap@xxxxxxxxxx>, Julien Grall <julien@xxxxxxx>, Wei Liu <wl@xxxxxxx>, xen-devel@xxxxxxxxxxxxxxxxxxxx
  • Delivery-date: Wed, 08 Nov 2023 08:24:53 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

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



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.