From owner-svn-src-head@freebsd.org Mon Nov 6 05:46:57 2017 Return-Path: Delivered-To: svn-src-head@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id AFC26E4E20F; Mon, 6 Nov 2017 05:46:57 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from mail105.syd.optusnet.com.au (mail105.syd.optusnet.com.au [211.29.132.249]) by mx1.freebsd.org (Postfix) with ESMTP id 30492F98; Mon, 6 Nov 2017 05:46:56 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from [192.168.0.104] (c110-21-101-228.carlnfd1.nsw.optusnet.com.au [110.21.101.228]) by mail105.syd.optusnet.com.au (Postfix) with ESMTPS id 0EA31104258B; Mon, 6 Nov 2017 16:46:54 +1100 (AEDT) Date: Mon, 6 Nov 2017 16:46:53 +1100 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: Mateusz Guzik cc: Konstantin Belousov , Ian Lepore , src-committers , "Conrad E. Meyer" , "svn-src-all@freebsd.org" , Andriy Gapon , "svn-src-head@freebsd.org" , Warner Losh Subject: Re: svn commit: r325386 - head/sys/kern In-Reply-To: Message-ID: <20171106162139.Q987@besplex.bde.org> References: <20171105130607.GA2566@kib.kiev.ua> <20171105173032.GE2566@kib.kiev.ua> <20171105190214.GG2566@kib.kiev.ua> <20f694b3-c60c-1b6d-76a1-2ef14cbdd698@FreeBSD.org> <1509910670.99235.70.camel@freebsd.org> <20171105201503.GH2566@kib.kiev.ua> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed X-Optus-CM-Score: 0 X-Optus-CM-Analysis: v=2.2 cv=KeqiiUQD c=1 sm=1 tr=0 a=PalzARQSbocsUSjMRkwAPg==:117 a=PalzARQSbocsUSjMRkwAPg==:17 a=kj9zAlcOel0A:10 a=pGLkceISAAAA:8 a=WX_ER7Zxp3DyPIdJMc0A:9 a=CjuIK1q_8ugA:10 X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 06 Nov 2017 05:46:57 -0000 On Sun, 5 Nov 2017, Mateusz Guzik wrote: > On Sun, Nov 5, 2017 at 9:15 PM, Konstantin Belousov > 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( ""). 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