From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: redhat.com, ip: 209.132.183.28, mailfrom: lersek@redhat.com) Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by groups.io with SMTP; Thu, 05 Sep 2019 11:38:28 -0700 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 926601DA2; Thu, 5 Sep 2019 18:38:28 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-116-133.ams2.redhat.com [10.36.116.133]) by smtp.corp.redhat.com (Postfix) with ESMTP id 3FAF15D712; Thu, 5 Sep 2019 18:38:27 +0000 (UTC) From: "Laszlo Ersek" To: edk2-devel-groups-io Cc: Andrew Fish , Leif Lindholm , Michael D Kinney , Rebecca Cran Subject: [PATCH edk2-CCSS 3/3] must comment: add rule for documenting spurious variable assignments Date: Thu, 5 Sep 2019 20:38:20 +0200 Message-Id: <20190905183820.10312-4-lersek@redhat.com> In-Reply-To: <20190905183820.10312-1-lersek@redhat.com> References: <20190905183820.10312-1-lersek@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.6.2 (mx1.redhat.com [10.5.110.71]); Thu, 05 Sep 2019 18:38:28 +0000 (UTC) Content-Transfer-Encoding: quoted-printable Problem statement from Ard: > Sometimes, the GCC compiler warns about variables potentially being use= d > without having been initialized, while visual inspection reveals that > this is impossible. In such cases, we need to initialize such a variabl= e > to an arbitrary value only to avoid breaking the build, given our polic= y > to treat warnings as errors. In such cases we generally use LocalIntegerVariable =3D 0; and LocalPointerVariable =3D NULL; which takes care of the incorrect warning. However, it also makes the human analysis of any subsequent logic harder, because it suggests that assigning that specific zero or NULL value to the local variable is *required* by the subsequent logic. In order to highlight such assignments, whose sole purpose is to suppress invalid "use before init" warnings from compilers or static analysis tools, we should mandate comments such as: // // set LocalVariable to suppress incorrect compiler/analyzer warnings // LocalVariable =3D 0; (Magic values such as 0xDEADBEEF, which would obviate the necessity of explicit comments, have been considered, and rejected for stylistic reasons.) Cc: Andrew Fish Cc: Leif Lindholm Cc: Michael D Kinney Cc: Rebecca Cran Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3D607 Signed-off-by: Laszlo Ersek --- 6_documenting_software/64_what_you_must_comment.md | 39 ++++++++++++++++= ++++ README.md | 1 + 2 files changed, 40 insertions(+) diff --git a/6_documenting_software/64_what_you_must_comment.md b/6_docum= enting_software/64_what_you_must_comment.md index abb2114bf5bc..9e51c2e45816 100644 --- a/6_documenting_software/64_what_you_must_comment.md +++ b/6_documenting_software/64_what_you_must_comment.md @@ -58,3 +58,42 @@ instance differs. =20 When possible, you should also list the requirements that are satisfied = by the code. + +### 6.4.6 Comment spurious variable assignments. + +A compiler or static code analyzer may warn that an object with automati= c or +allocated storage duration is read without having been initialized, whil= e +visual inspection reveals that this is impossible. + +In order to suppress such a warning (which is emitted due to invalid dat= a flow +analysis), developers explicitly assign the affected object the value to= which +the same object would be initialized automatically, had the object stati= c +storage duration, and no initializer. (The value assigned could be arbit= rary; +the above-mentioned value is chosen for stylistic reasons.) For example: + +```c +UINTN LocalIntegerVariable; +VOID *LocalPointerVariable; + +LocalIntegerVariable =3D 0; +LocalPointerVariable =3D NULL; +``` + +This kind of assignment is difficult to distinguish from assignments whe= re the +initial value of an object is meaningful, and is consumed by other code = without +an intervening assignment. Therefore, each such assignment must be docum= ented, +as follows: + +```c +UINTN LocalIntegerVariable; +VOID *LocalPointerVariable; + +// +// set LocalIntegerVariable to suppress incorrect compiler/analyzer warn= ings +// +LocalIntegerVariable =3D 0; +// +// set LocalPointerVariable to suppress incorrect compiler/analyzer warn= ings +// +LocalPointerVariable =3D NULL; +``` diff --git a/README.md b/README.md index e26133540368..0648819f0d3a 100644 --- a/README.md +++ b/README.md @@ -113,3 +113,4 @@ Copyright (c) 2006-2017, Intel Corporation. All right= s reserved. | 2.2 | Convert to Gitbook = = | June 2017 | | | [#425](https://bugzilla.tianocore.org/show_bug.cgi?id=3D425= ) [CCS] clarify line breaking and indentation requirements for multi-line= function calls | | | | [#1656](https://bugzilla.tianocore.org/show_bug.cgi?id=3D16= 56) Update all Wiki pages for the BSD+Patent license change with SPDX ide= ntifiers | | +| | [#607](https://bugzilla.tianocore.org/show_bug.cgi?id=3D607= ) Document code comment requirements for spurious variable assignments = | | --=20 2.19.1.3.g30247aa5d201