Date: Tue, 19 Aug 2008 22:28:36 +0200 From: Hans Petter Selasky <hselasky@c2i.net> To: Alfred Perlstein <alfred@freebsd.org> Cc: freebsd-usb@freebsd.org Subject: Re: usb4bsd patch review [was Re: ...] Message-ID: <200808192228.39015.hselasky@c2i.net> In-Reply-To: <20080819200017.GC16977@elvis.mu.org> References: <20080818205914.GJ16977@elvis.mu.org> <48AB233C.2010602@FreeBSD.org> <20080819200017.GC16977@elvis.mu.org>
next in thread | previous in thread | raw e-mail | index | archive | help
> >
> > What do the newbus guys say about this? Adding a workaround in
> > underlying code for a problem caused by your own code is often a signal
> > that you're not going about it the right way. At the very least the
> > reason for the special case should be documented here.
>
> I need to think about this, Hans gave me a better argument on
> AIM earlier for it, I need to reload this in my head.
>
> > >int
> > >device_delete_all_children(device_t dev)
> > >{
> > > device_t *devlist;
> > > int devcount;
> > > int error;
> > >
> > > error = device_get_children(dev, &devlist, &devcount);
> > > if (error == 0) {
> > > while (devcount-- > 0) {
> > > error = device_delete_child(dev,
> > > devlist[devcount]);
> > > if (error) {
> > > break;
> > > }
> > > }
> > > free(devlist, M_TEMP);
> > > }
> > > return (error);
> > >}
> > >
In the existing kernel code, "device_get_children()" is used many places
without checking the error code. I have patches, but they are not part of the
patch-set.
Also freeing a pointer to zero bytes is not logical. I'm not sure if this is
allowed in kernel space?
ptr = malloc(0, ... )
free(0);
The device_get_children() could have returned an error if there are no
children, but again, the existing code does not check this return value.
--HPS
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200808192228.39015.hselasky>