[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <Zw6uYl0CXhl_ru08@smile.fi.intel.com>
Date: Tue, 15 Oct 2024 21:03:14 +0300
From: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
To: Doug Anderson <dianders@...omium.org>
Cc: Chen-Yu Tsai <wenst@...omium.org>, Rob Herring <robh@...nel.org>,
Saravana Kannan <saravanak@...gle.com>,
Matthias Brugger <matthias.bgg@...il.com>,
AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>,
Wolfram Sang <wsa@...nel.org>, Benson Leung <bleung@...omium.org>,
Tzung-Bi Shih <tzungbi@...nel.org>, chrome-platform@...ts.linux.dev,
devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
linux-mediatek@...ts.infradead.org, linux-kernel@...r.kernel.org,
Johan Hovold <johan@...nel.org>, Jiri Kosina <jikos@...nel.org>,
linux-i2c@...r.kernel.org
Subject: Re: [PATCH v8 4/8] i2c: Introduce OF component probe function
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@...omium.org> 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
Powered by blists - more mailing lists