On 8/16/19 8:02 PM, Jacek Anaszewski wrote: > On 8/16/19 1:49 PM, Pavel Machek wrote: >> Hi! >> >>> If in the certain driver the LED is optional, and it's a majority of them, >>> the call of led_classdev_unregister() still requires some additional checks. >>> >>> The usual pattern on unregistering is to check for NULL, but we also check >>> for IS_ERR() in case device_create_with_groups() fails. >>> >>> The change will reduce a burden in a lot of drivers to repeatedly check >>> for above conditions. >> >> I don't see majority of calls being protected. Doing nothing on NULL >> sounds reasonable. I'm less sure about "IS_ERR"... > > device_create_groups_vargs() returns ERR_PTR(retval) on error so > led_cdev->dev may be left non-NULL even on error. > > We can be correct checking only for NULL if we will add > "led_cdev->dev = NULL" assignment in "if (IS_ERR(led_cdev->dev)" branch > in led_classdev_register_ext(). Nonetheless I'm personally in favor of > Andy's solution. > Patch applied, thanks. -- Best regards, Jacek Anaszewski
