Date: Tue, 19 Mar 2024 13:43:04 +0000 (UTC) From: "Bjoern A. Zeeb" <bzeeb-lists@lists.zabbadoz.net> To: hackers@freebsd.org Subject: Re: Why I dislike MPASS... Message-ID: <261pq217-nqq5-9snn-r407-oo895s6843ss@yvfgf.mnoonqbm.arg> In-Reply-To: <E92DF0F3-54E4-4F08-ADE7-551C94F3C4FA@FreeBSD.org> References: <57o4rnnq-013s-3nsn-59n5-4ssn1pq81s94@yvfgf.mnoonqbm.arg> <E92DF0F3-54E4-4F08-ADE7-551C94F3C4FA@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
This message is in MIME format. The first part should be readable text, while the remaining parts are likely unreadable without MIME-aware tools. --1098556516-1809024439-1710855785=:3455 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8BIT On Tue, 19 Mar 2024, Kristof Provost wrote: > On 19 Mar 2024, at 8:46, Bjoern A. Zeeb wrote: >> Hi, >> >> needless to say, this needed a 2nd kernel, a reproducer (when no gdb and >> no crash dump was avail). >> >> XXX-BZ KABOOM 45000 > 0 && 45000 <= 360 && 45000 < 1536 >> panic: Assertion (key.to) > 0 && (key.to) <= w_max_used_index && (key.to) < >> witness_count failed at /usr/src/sys/kern/subr_witness.c:3118 >> >> This is why I dislike MPASS; if it hits you are often no wiser as to >> why, especially if people added more than a single condition. >> >> Why do we make our life harder ourselves by not having informative >> messages (I mean that is what the comment above MPASS claims)? >> >> I still have no idea how I get to that assertion but at least key.to >> looks dodgy enough so that I can go look where-as before I really didn't >> know if I had hit a limit or which one... >> > Presumably you’d prefer something that’d log the actual compared values? Yes, because without them you still don't know what's wrong. > E.g. something like `ASSERT_GT(key.to, 0); ASSERT_LTE(key.to, > w_max_used_index); ASSERT_LT(key.to, witness_count);`? > I vaguely recall seeing discussion of similar constructs in the ZFS code. > > I’d enthusiastically support adding, and encouraging the use of, macros like > that. > > I’d object to removing or discouraging MPASS(), because I fear that will > reduce the number of assertions developers add to the code, and we’d be > objectively worse off in a world where the assertion was ` ` rather than > `MPASS((key.to) > 0 && (key.to) <= w_max_used_index && (key.to) < > witness_count);`. > > We should probably encourage people to avoid complex expressions in MPASS(), > but I’d still much rather have the complex expression than nothing at all. Yes, MPASS should probably only be used for a single simple expression the most. And I am all for assertions as they are great documentation of assumption in code as well. It's the first stage of writing test code as well ;-) But using KASSERT with a proper message which isn't a teapot is too hard and too time consuming? Always think of when you get a PR from a user (not a machine you sit in front of) what can you do with this information and what's the first things you'd love to know. I often find that the latter changes as code changes or one has a better understanding of a problem (depending on the complexity of the problem). But I also always find that if I didn't have the initial extra information I would have to ask for it often by guessing the best three things... or if ddb is avail at least can have something looked up "type show ifnet <values from message> and send that output along". /bz -- Bjoern A. Zeeb r15:7 --1098556516-1809024439-1710855785=:3455--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?261pq217-nqq5-9snn-r407-oo895s6843ss>