git: 83a426d13a6a - main - KASSERT(9): add assertion message guidelines

From: Mitchell Horne <mhorne_at_FreeBSD.org>
Date: Thu, 21 Mar 2024 15:25:28 UTC
The branch main has been updated by mhorne:

URL: https://cgit.FreeBSD.org/src/commit/?id=83a426d13a6a384e63e75d8252c03dd40af3817e

commit 83a426d13a6a384e63e75d8252c03dd40af3817e
Author:     Mitchell Horne <mhorne@FreeBSD.org>
AuthorDate: 2024-03-21 14:54:49 +0000
Commit:     Mitchell Horne <mhorne@FreeBSD.org>
CommitDate: 2024-03-21 15:24:16 +0000

    KASSERT(9): add assertion message guidelines
    
    Add some text describing how to create useful assertion messages.
    Improve and add to the EXAMPLES.
    
    See the discussion prompting this on -hackers:
    https://mail-archive.freebsd.org/cgi/mid.cgi?57o4rnnq-013s-3nsn-59n5-4ssn1pq81s94
    
    Reviewed by:    emaste
    Discussed with: imp, bz
    MFC after:      1 week
    Sponsored by:   The FreeBSD Foundation
    Differential Revision:  https://reviews.freebsd.org/D44434
---
 share/man/man9/KASSERT.9 | 72 ++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 67 insertions(+), 5 deletions(-)

diff --git a/share/man/man9/KASSERT.9 b/share/man/man9/KASSERT.9
index 1d28ce80c9fb..8d9dd98014a8 100644
--- a/share/man/man9/KASSERT.9
+++ b/share/man/man9/KASSERT.9
@@ -2,7 +2,7 @@
 .\"
 .\" Copyright (c) 2000 Jonathan M. Bresler
 .\" All rights reserved.
-.\" Copyright (c) 2023 The FreeBSD Foundation
+.\" Copyright (c) 2023-2024 The FreeBSD Foundation
 .\"
 .\" Portions of this documentation were written by Mitchell Horne
 .\" under sponsorship from the FreeBSD Foundation.
@@ -29,7 +29,7 @@
 .\" (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
 .\" THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 .\"
-.Dd March 16, 2023
+.Dd March 19, 2024
 .Dt KASSERT 9
 .Os
 .Sh NAME
@@ -93,8 +93,37 @@ The
 macro (read as: "must-pass")
 is a convenience wrapper around
 .Fn KASSERT
-that automatically generates a sensible assertion message including file and
-line information.
+that automatically generates a simple assertion message including file and line
+information.
+.Ss Assertion Guidelines
+When adding new assertions, keep in mind their primary purpose: to aid in
+identifying and debugging of complex error conditions.
+.Pp
+The panic messages resulting from assertion failures should be useful without
+the resulting kernel dump; the message may be included in a bug report, and
+should contain the relevant information needed to discern how the assertion was
+violated.
+This is especially important when the error condition is difficult or
+impossible for the developer to reproduce locally.
+.Pp
+Therefore, assertions should adhere to the following guidelines:
+.Bl -enum
+.It
+Whenever possible, the value of a runtime variable checked by an assertion
+condition should appear in its message.
+.It
+Unrelated conditions must appear in separate assertions.
+.It
+Multiple related conditions should be distinguishable (e.g. by value), or split
+into separate assertions.
+.It
+When in doubt, print more information, not less.
+.El
+.Pp
+Combined, this gives greater clarity into the exact cause of an assertion
+panic; see
+.Sx EXAMPLES
+below.
 .Sh EXAMPLES
 A hypothetical
 .Vt struct foo
@@ -106,11 +135,19 @@ foo_dealloc(struct foo *fp)
 {
 
 	KASSERT((fp->foo_flags & FOO_ACTIVE) == 0,
-	    ("%s: fp %p is still active", __func__, fp));
+	    ("%s: fp %p is still active, flags=%x", __func__, fp,
+	    fp->foo_flags));
 	...
 }
 .Ed
 .Pp
+This assertion provides the full flag set for the object, as well as the memory
+pointer, which may be used by a debugger to examine the object in detail
+.Po
+for example with a 'show foo' command in
+.Xr ddb 4
+.Pc .
+.Pp
 The assertion
 .Bd -literal -offset indent
 MPASS(td == curthread);
@@ -121,6 +158,31 @@ message:
 .Bd -literal -offset indent
 panic: Assertion td == curthread failed at foo.c:87
 .Ed
+.Pp
+This is a simple condition, and the message provides enough information to
+investigate the failure.
+.Pp
+The assertion
+.Bd -literal -offset indent
+MPASS(td == curthread && (sz >= SIZE_MIN && sz <= SIZE_MAX));
+.Ed
+.Pp
+is
+.Em NOT
+useful enough.
+The message doesn't indicate which part of the assertion was violated, nor
+does it report the value of
+.Dv sz ,
+which may be critical to understanding
+.Em why
+the assertion failed.
+.Pp
+According to the guidelines above, this would be correctly expressed as:
+.Bd -literal -offset indent
+MPASS(td == curthread);
+KASSERT(sz >= SIZE_MIN && sz <= SIZE_MAX,
+    ("invalid size argument: %u", sz));
+.Ed
 .Sh SEE ALSO
 .Xr panic 9
 .Sh AUTHORS