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. -- Best regards, Jacek Anaszewski
