Date: Mon, 6 Nov 2017 16:46:53 +1100 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Mateusz Guzik <mjguzik@gmail.com> Cc: Konstantin Belousov <kostikbel@gmail.com>, Ian Lepore <ian@freebsd.org>, src-committers <src-committers@freebsd.org>, "Conrad E. Meyer" <cem@freebsd.org>, "svn-src-all@freebsd.org" <svn-src-all@freebsd.org>, Andriy Gapon <avg@freebsd.org>, "svn-src-head@freebsd.org" <svn-src-head@freebsd.org>, Warner Losh <imp@bsdimp.com> Subject: Re: svn commit: r325386 - head/sys/kern Message-ID: <20171106162139.Q987@besplex.bde.org> In-Reply-To: <CAGudoHGN498vQ2NrbKd9wGJjCV-0WV7ESs3-x5YQyTvoOejbmA@mail.gmail.com> References: <CAG6CVpUaXSX26Bc839kn96EXnUjtGyQ3_eNJPhsRB%2Bv6G6gH1Q@mail.gmail.com> <20171105130607.GA2566@kib.kiev.ua> <CAG6CVpV3SfV8VuNeMJEuN%2BMSu3424mK=HO_-YW9vRt9HEdcJZQ@mail.gmail.com> <20171105173032.GE2566@kib.kiev.ua> <CAG6CVpVQ4RSgz883ezsdExVDQ4RbSiR_y%2BZ5XWDNFeYBpMS-cQ@mail.gmail.com> <CANCZdfqApxd99G-HqVfe4x8EysYJVCAcHksqTeUuBAaYR1OjEA@mail.gmail.com> <20171105190214.GG2566@kib.kiev.ua> <20f694b3-c60c-1b6d-76a1-2ef14cbdd698@FreeBSD.org> <CAG6CVpUuQ-LP9P0ckkkv4Y07aowEhT3qj8e45AK-Nnzhh_LnMA@mail.gmail.com> <1509910670.99235.70.camel@freebsd.org> <20171105201503.GH2566@kib.kiev.ua> <CAGudoHGN498vQ2NrbKd9wGJjCV-0WV7ESs3-x5YQyTvoOejbmA@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Sun, 5 Nov 2017, Mateusz Guzik wrote:
> On Sun, Nov 5, 2017 at 9:15 PM, Konstantin Belousov <kostikbel@gmail.com>
> wrote:
>
>> On Sun, Nov 05, 2017 at 12:37:50PM -0700, Ian Lepore wrote:
>>> IMO, the only reason ASSERT-style macros exist is to hide the
>>> conditional-on-build-type part of the operation. That is, to avoid
>>> having #ifdef INVARIANTS scattered everywhere.
>> bde' point is that KASSERT() is badly designed, and I agree with him.
>> Now we could at least remove the () around the message formatting part,
>> but it is too late.
>>>
>>> Creating a macro to generate always-on error detection and reporting
>>> code just because there exists a macro to do so conditionally seems to
>>> turn the world on its head.
>> I agree with this statement. if()panic(); construct is good enough, IMO.
I agree.
> I don't like our panic messages whatsoever, they are quite often not
> informative.
I don't like our panic messages, since they are too long.
> For instance consider:
> if (obj->foo < bar)
> panic("bad foo %d, have fun looking for bar");
I almost prefer panic(".") where the dot is rendered in red :-). The
dot is too hard to grep for, so I acually prefer panic(
"<literal_function_name>"). panic("%s", __func__) is even better than
the dot for unreadability.
Often the value of foo and bar and hundreds of other variables printed by
a bloated panic statement are obvious or uninteresting, and you need to
use a debugger and a compiler that does't optimize out or otherwise
obfuscate anything.
> Instead a macro akin to PASS(obj->foo, <, bar, "obj %p", obj); can
> expand itself to stringify the first 3 terms and also show the compared
> values. Saves on boiler-plate written by hand.
>
> I think *all* panics should be accompanied with a linux's oops-like dump.
Dumps can be produced by debuggers or hd on core files. More than function
names (also offsets within functions) can be produced by debuggers or
addr2line, all without any space/time bloat for stringification.
Some panics already occur via traps. Then trap() does a simple dump
and panic() can't print anything more useful. Instead of the red dot
panic, I also prefer a breakpoint or null pointer trap. Both are
restartble using kdb, unlike panic(). If no backend is attached to kdb,
then these traps turn into panics with a dump from trap().
Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20171106162139.Q987>