Re: [PATCH] fix error handling in bus_add_device

[Greg KH]

On Wed, May 18, 2005 at 09:19:37AM +0200, Hannes Reinecke wrote:
> Greg KH wrote:
> > On Thu, May 12, 2005 at 04:19:24PM +0200, Hannes Reinecke wrote:
> >>Hi Greg,
> >>
> >>this patch fixes the error handling in bus_add_device() and
> >>device_attach(). Previously it was 'interesting'.
> >>And totally confusing to boot.
> > 
> > I agree, that's why it has been rewritten in the -mm tree :)
> > 
> > Anyway, your patch doesn't take into account that device_attach()'s
> > return value is tested in the bus_rescan_devices_helper(), so if you
> > change the return value, that also needs to be changed.
> > 
> > But even in the -mm tree, the bus_add_devices() function has not had the
> > error handling added to it that you provided, is there any devices that
> > you are seeing that need this?
> > 
> Not yet :-)
> 
> I'm just doing some cleanups here which me and Kay Sievers will be
> exploiting in the near future.
> My main point is:
> either we do an error check in bus_add_device and return a proper
> status, or we don't and fix bus_add_device to be of type 'void'.
> And as some functions called by bus_add_device may fail I thought it
> reasonable to evaluate the return status properly.
> Unless you tell me that bus_add_device is a fire-and-forget procedure
> and we don't care at all for any failures. But then we should at least
> set the type of bus_add_device() to 'void'.
> You're the maintainer, you have to decide :-).
> I don't care either way, I just want to have it consistent.
> 
> But you're correct about the bus_rescan_devices_helper. Fixed and new
> patch attached.

Ok, I agree that we should have error checks in there.  Now, could you
make your patch against the latest -mm tree instead due to all of the
changes involved in that area in my trees?  That way I can apply it :)

thanks,

greg k-h
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/