On Tue Feb 11, 2025 at 3:17 AM -05, Greg Kroah-Hartman wrote: > On Tue, Feb 11, 2025 at 02:43:20AM -0500, Kurt Borja wrote: >> On Tue Feb 11, 2025 at 2:33 AM -05, Greg Kroah-Hartman wrote: >> > On Tue, Feb 11, 2025 at 08:27:26AM +0100, Greg Kroah-Hartman wrote: >> >> On Mon, Feb 10, 2025 at 12:56:46PM -0500, Kurt Borja wrote: >> >> I'll work on adding "if probe failed, don't let the device be created" >> >> logic as it's a simple change, BUT it is a functionally different change >> >> from what the current api that this code is replacing is doing (i.e. the >> >> current platform device creation code does this today and no one has >> >> ever hit this in their use of it in the past few decades.) >> > >> > How about something as simple as this change, does that provide what you >> > are thinking about here? Only compile tested, not runtime tested at >> > all: >> >> Yes, LGTM. However dev->driver is set to NULL if the probe fails so >> wouldn't >> >> if (!dev->driver) >> >> do the job? > > True, that would work, and be much simpler, I'll go add that AND > actually test it :) Nice :) > >> I understand your point about groups. This of course solves it, although >> isn't there a small windows between device_add() and device_destroy() in >> the failed probe path, in which a show/store/visibility method could >> dereference a NULL drvdata? > > Ok, I looked it up and it turns out that the groups wouldn't have even > been created if probe() failed (see the call to call_driver_probe() in > really_probe() in drivers/base/dd.c) So all should be good here. Are you refering to this line [1]? Because those are the driver's dev_groups. dev->groups are added by device_add_attrs() in device_add(). Here is the line [2]. This happens *way* before the device is added to the bus. Also I tested with a sample faux device (faux faux device :)) and the groups do get added, even if the probe fails. [1] https://elixir.bootlin.com/linux/v6.14-rc1/source/drivers/base/core.c#L2887 [2] https://elixir.bootlin.com/linux/v6.14-rc1/source/drivers/base/core.c#L2887 -- ~ Kurt > > thanks, > > greg k-h
