On Sun, 7 Jan 2024 22:05:35 +0100 Rasmus Villemoes <linux@xxxxxxxxxxxxxxxxxx> wrote: > On 06/01/2024 18.53, Jonathan Cameron wrote: > > > > ething 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? > > IIRC, it's for code generation, avoiding emitting the call to the > cleanup function on the code paths where the compiler knows the argument > is NULL. And on the other return paths, the compiler most likely knows > the value is non-NULL, so the conditional is elided (but of course not > the put call). Read ERR_OR_NULL as appropriate. > > Rasmus > > Thanks. Makes sense. A reference in another thread where this was being discussed lead me to a reference to Linus arguing for just this: https://lore.kernel.org/all/20230814161731.GN776869@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx/