device_detach() never clears devclasses

[John Baldwin]

I haven't reproduced this, I just have a hunch based on code-reading while
fixing another bug.  Ah, looking in SVN history it looks like this was broken
in 129711 when device_attach() was split out of device_probe_and_attach().  
Specifically, if DEVICE_ATTACH() fails then this:

                /* Unset the class; set in device_probe_child */
                if (dev->devclass == NULL)
                        (void)device_set_devclass(dev, NULL);

is wrong.  dev->devclass is always non-NULL at this point.  However, we should
be clearing the devclass for any device that doesn't have a fixed device
class.  And in fact that is what device_detach() does:

        if (!(dev->flags & DF_FIXEDCLASS))
                devclass_delete_device(dev->devclass, dev);

The patch below fixes this:

Index: subr_bus.c
===================================================================
--- subr_bus.c (revision 232218)
+++ subr_bus.c (working copy)
@@ -2732,9 +2732,8 @@ device_attach(device_t dev)
  if ((error = DEVICE_ATTACH(dev)) != 0) {
  printf("device_attach: %s%d attach returned %d\n",
     dev->driver->name, dev->unit, error);
- /* Unset the class; set in device_probe_child */
- if (dev->devclass == NULL)
- (void)device_set_devclass(dev, NULL);
+ if (!(dev->flags & DF_FIXEDCLASS))
+ devclass_delete_device(dev->devclass, dev);
  (void)device_set_driver(dev, NULL);
  device_sysctl_fini(dev);
  dev->state = DS_NOTPRESENT;


--
John Baldwin
_______________________________________________
[hidden email] mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-new-bus
To unsubscribe, send any mail to "[hidden email]"

[Warner Losh]

This looks good to me.  I think I should have had != here anyway. :)

Warner

On Feb 27, 2012, at 10:08 AM, John Baldwin wrote:

> I haven't reproduced this, I just have a hunch based on code-reading while
> fixing another bug.  Ah, looking in SVN history it looks like this was broken
> in 129711 when device_attach() was split out of device_probe_and_attach().  
> Specifically, if DEVICE_ATTACH() fails then this:
>
> /* Unset the class; set in device_probe_child */
> if (dev->devclass == NULL)
> (void)device_set_devclass(dev, NULL);
>
> is wrong.  dev->devclass is always non-NULL at this point.  However, we should
> be clearing the devclass for any device that doesn't have a fixed device
> class.  And in fact that is what device_detach() does:
>
> if (!(dev->flags & DF_FIXEDCLASS))
> devclass_delete_device(dev->devclass, dev);
>
> The patch below fixes this:
>
> Index: subr_bus.c
> ===================================================================
> --- subr_bus.c (revision 232218)
> +++ subr_bus.c (working copy)
> @@ -2732,9 +2732,8 @@ device_attach(device_t dev)
> if ((error = DEVICE_ATTACH(dev)) != 0) {
> printf("device_attach: %s%d attach returned %d\n",
>    dev->driver->name, dev->unit, error);
> - /* Unset the class; set in device_probe_child */
> - if (dev->devclass == NULL)
> - (void)device_set_devclass(dev, NULL);
> + if (!(dev->flags & DF_FIXEDCLASS))
> + devclass_delete_device(dev->devclass, dev);
> (void)device_set_driver(dev, NULL);
> device_sysctl_fini(dev);
> dev->state = DS_NOTPRESENT;
>
>
> --
> John Baldwin
>
>

_______________________________________________
[hidden email] mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-new-bus
To unsubscribe, send any mail to "[hidden email]"