On Tue, Oct 15, 2024 at 10:58:31AM -0700, Doug Anderson wrote: > On Tue, Oct 8, 2024 at 12:35 AM Chen-Yu Tsai <wenst@xxxxxxxxxxxx> wrote: ... > > + struct device_node *i2c_node __free(device_node) = i2c_of_probe_get_i2c_node(dev, type); > > + if (IS_ERR(i2c_node)) > > + return PTR_ERR(i2c_node); > > I'm still getting comfortable with the __free() syntax so sorry if I'm > wrong, but I _think_ the above is buggy. I believe that the definition > of the free function for "device_node" is from: > > DEFINE_FREE(device_node, struct device_node *, if (_T) of_node_put(_T)) > > ...which means it'll call of_node_put() to free "i2c_node" when it > goes out of scope. of_node_put() handles NULL pointers but _not_ ERR > pointers. So I think that if you get an error back and then return via > the PTR_ERR(i2c_node) then it'll crash because it will try to free an > ERR pointer. Did I get that right? Presumably you need to instead do: > > return PTR_ERR(no_free_ptr(i2c_node)); > > ...or change of_node_put() to be a noop for error pointers? I could state that device_node FREE class has to be updated for these cases. fwnode, for example, handles both error pointers and NULL. -- With Best Regards, Andy Shevchenko
