Re: patch: debug instrumentation for dev/raidframe/rf_netbsdkintf.c

2019-01-22 Thread Christos Zoulas
In article <20190122173641.ga26...@irregular-apocalypse.k.bsd.de>, Christoph Badura wrote: >On Mon, Jan 21, 2019 at 11:44:04PM +0100, Christoph Badura wrote: >> On Tue, Jan 22, 2019 at 08:32:48AM +1100, matthew green wrote: >> > > > @@ -472,6 +472,9 @@ >> > > >const char *bootname =

Re: patch: debug instrumentation for dev/raidframe/rf_netbsdkintf.c

2019-01-22 Thread Christoph Badura
On Mon, Jan 21, 2019 at 11:44:04PM +0100, Christoph Badura wrote: > On Tue, Jan 22, 2019 at 08:32:48AM +1100, matthew green wrote: > > > > @@ -472,6 +472,9 @@ > > > > const char *bootname = device_xname(bdv); > > > > size_t len = strlen(bootname); > > > > > > > > + if (bdv

Re: patch: debug instrumentation for dev/raidframe/rf_netbsdkintf.c

2019-01-21 Thread Christos Zoulas
In article <24279.1548106...@splode.eterna.com.au>, matthew green wrote: >> > @@ -472,6 +472,9 @@ >> >const char *bootname = device_xname(bdv); >> >size_t len = strlen(bootname); >> > >> > + if (bdv == NULL) >> > + return 0; >> > + >> >> This looked suspicious, even before I

Re: patch: debug instrumentation for dev/raidframe/rf_netbsdkintf.c

2019-01-21 Thread Christoph Badura
On Tue, Jan 22, 2019 at 08:32:48AM +1100, matthew green wrote: > > > @@ -472,6 +472,9 @@ > > > const char *bootname = device_xname(bdv); > > > size_t len = strlen(bootname); > > > > > > + if (bdv == NULL) > > > + return 0; > > > + > > > > This looked suspicious, even before I read

re: patch: debug instrumentation for dev/raidframe/rf_netbsdkintf.c

2019-01-21 Thread matthew green
> >>> @@ -472,6 +472,9 @@ > >>> const char *bootname = device_xname(bdv); > >>> size_t len = strlen(bootname); > >>> > >>> + if (bdv == NULL) > >>> + return 0; > >>> + > >> > >> This looked suspicious, even before I read the code. > >> > >> The question is if it is ever legitimate for

re: patch: debug instrumentation for dev/raidframe/rf_netbsdkintf.c

2019-01-21 Thread Paul Goyette
On Tue, 22 Jan 2019, matthew green wrote: @@ -472,6 +472,9 @@ const char *bootname = device_xname(bdv); size_t len = strlen(bootname); + if (bdv == NULL) + return 0; + This looked suspicious, even before I read the code. The question is if it is ever

Re: patch: debug instrumentation for dev/raidframe/rf_netbsdkintf.c

2019-01-21 Thread Greg Troxel
Christoph Badura writes: > On Mon, Jan 21, 2019 at 04:24:49PM -0500, Greg Troxel wrote: >> Separetaly from debug code being careful, if it's a rule that bdv can't >> be NULL, it's just as well to put in a KASSERT. Then we'll find out >> where that isn't true and can fix it. > > I must not be

re: patch: debug instrumentation for dev/raidframe/rf_netbsdkintf.c

2019-01-21 Thread matthew green
> > @@ -472,6 +472,9 @@ > > const char *bootname = device_xname(bdv); > > size_t len = strlen(bootname); > > > > + if (bdv == NULL) > > + return 0; > > + > > This looked suspicious, even before I read the code. > > The question is if it is ever legitimate for bdv to be

Re: patch: debug instrumentation for dev/raidframe/rf_netbsdkintf.c

2019-01-21 Thread Christoph Badura
On Mon, Jan 21, 2019 at 04:24:49PM -0500, Greg Troxel wrote: > Separetaly from debug code being careful, if it's a rule that bdv can't > be NULL, it's just as well to put in a KASSERT. Then we'll find out > where that isn't true and can fix it. I must not be getting something. If

Re: patch: debug instrumentation for dev/raidframe/rf_netbsdkintf.c

2019-01-21 Thread Greg Troxel
Christoph Badura writes: >> > + if (bdv == NULL) >> > + return 0; >> > + >> >> This looked suspicious, even before I read the code. >> The question is if it is ever legitimate for bdv to be NULL. > > That is an excellent point. The short answer is, no it isn't. And it > never was

Re: patch: debug instrumentation for dev/raidframe/rf_netbsdkintf.c

2019-01-21 Thread Christoph Badura
On Mon, Jan 21, 2019 at 06:36:37PM +0100, Christoph Badura wrote: > I think the following is better. Compile-tested only for both #ifdef > conditions, but I think that is OK. Ugh. I forgot to put a comment on that function. How about this: /* * Provide a wrapper around rf_containsboot that

Re: patch: debug instrumentation for dev/raidframe/rf_netbsdkintf.c

2019-01-21 Thread Christoph Badura
On Mon, Jan 21, 2019 at 10:42:10AM -0500, Greg Troxel wrote: > Christoph Badura writes: > > Index: rf_netbsdkintf.c > > === > > RCS file: /cvsroot/src/sys/dev/raidframe/rf_netbsdkintf.c,v > > retrieving revision 1.356 > > diff -u

Re: patch: debug instrumentation for dev/raidframe/rf_netbsdkintf.c

2019-01-21 Thread Greg Troxel
Christoph Badura writes: > Here is some instrumentation I found useful during my recent debugging. > If there are no objections, I'd like to commit soon. > > The change to rf_containsroot() simplifies the second DPRINTF that I added. > > Index: rf_netbsdkintf.c >