Date: Fri, 20 Mar 2009 09:42:32 -0700 From: Marcel Moolenaar <xcllnt@mac.com> To: Marius Strobl <marius@alchemy.franken.de> Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r190105 - head/sys/sparc64/sparc64 Message-ID: <458289DF-A730-453D-81D3-DAB8631A9F34@mac.com> In-Reply-To: <20090320163359.GH59320@alchemy.franken.de> References: <200903192040.n2JKeoYY075200@svn.freebsd.org> <31910B64-3437-4C1D-8234-FC6A1C3D4F8B@mac.com> <20090320133648.GE59320@alchemy.franken.de> <BC840CB3-CCD0-41D6-BCA4-4F5FD7B2A83A@mac.com> <20090320163359.GH59320@alchemy.franken.de>
next in thread | previous in thread | raw e-mail | index | archive | help
On Mar 20, 2009, at 9:33 AM, Marius Strobl wrote: >> >>>>> Log: >>>>> There's no need to wrap kdb_enter() in #ifdef KDB as it's always >>>>> available. >>>> >>>> That's not quite how it works. >>>> >>>> option KDB is used to build the kernel with debugging features >>>> that could impact performance, security and/or functionality. >>>> In this case it's not so much a matter of whether kdb_enter() >>>> is defined or not, but rather whether the kernel should respect >>>> -d. >>>> >>> >>> That's generally true but the places where I removed >>> #ifdef KDB don't have an impact on security, performance >>> doesn't matter and -d still does nothing if there's no >>> debugger available (which in turn would require options >>> KDB, at least according to documentation). I'm not sure >>> what your're actually trying to say; following your >>> logic strictly would mean that subr_kdb.c shouldn't be >>> standard but only compiled in when options KDB is >>> present. >> >> The functions in subr_kdb.c have been made standard (i.e >> non-optional) so that they can actually be used safely >> from modules without having to worry about whether the >> kernel has some option enabled. So, option KDB doesn't >> control their visibility. >> >> A debugger is present when either DDB or GDB is defined. >> So, option KDB doesn't control that either. >> >> Option KDB doesn't do much. For the most part it guards >> code that results in entry into the debugger. > > Okay, I now see why you objected to r190105. It wasn't an objection, just a remark. > My point > is that in this case the code in question doesn't > need such protection (just as a module not using KDB > wouldn't have) as there's no relevant impact on > functionality, performance or security here but > especially in r190106 removing it improves readability. My only real objection is that this creates an inconsistency between platforms. By removing option KDB the kernel will not respect -d (or boot_kdb=1) on any platform but sparc64. There's a user-visible impact. I would prefer that we keep all platforms the same so that documentation can apply uniformly and isn't only applicable to i386. I'm perfectly happy if uniformity is achieved by removing the ifdef from all platforms :-) Anyway: it's not a big deal in any case. -- Marcel Moolenaar xcllnt@mac.com
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?458289DF-A730-453D-81D3-DAB8631A9F34>