On 6 January 2024 15:16:53 GMT, Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx> wrote: >On Mon, Jan 01, 2024 at 05:25:59PM +0000, Jonathan Cameron wrote: >> From: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx> >> >> This allows the following >> >> struct fwnode_handle *child __free(kfree) = NULL; >> >> device_for_each_child_node(dev, child) { >> if (false) >> return -EINVAL; >> } >> >> without the fwnode_handle_put() call which tends to complicate early >> exits from such loops and lead to resource leak bugs. >> >> Can also be used where the fwnode_handle was obtained from a call >> such as fwnode_find_reference() as it will safely do nothing if >> IS_ERR() is true. > >... > >> struct fwnode_handle *fwnode_handle_get(struct fwnode_handle *fwnode); >> void fwnode_handle_put(struct fwnode_handle *fwnode); >> +DEFINE_FREE(fwnode_handle, struct fwnode_handle *, fwnode_handle_put(_T)) > >In GPIO we have something similar and PeterZ explained there why if (_T) is >important, hence this should be I can't find the reference unfortunately. > >DEFINE_FREE(fwnode_handle, struct fwnode_handle *, if (_T) fwnode_handle_put(_T)) > >or even > >DEFINE_FREE(fwnode_handle, struct fwnode_handle *, if (!IS_ERR_OR_NULL(_T)) fwnode_handle_put(_T)) > >as we accept in many calls an error pointer as unset / undefined firmware node >handle. The function called has a protection for null and error inputs so I'm not sure why extra protection is needed? J > >