Re: [PATCH v2 1/2] software node: fix refcount leak in software_node_get_next_child()

[Andy Shevchenko <andriy.shevchenko@linux.intel.com>]

On Thu, Jun 04, 2026 at 07:15:26PM +0800, Xu Yang wrote:
> On Wed, Jun 03, 2026 at 12:40:27PM +0300, Andy Shevchenko wrote:
> > On Wed, Jun 03, 2026 at 04:44:31PM +0800, Xu Yang wrote:

...

> > >  	struct swnode *p = to_swnode(fwnode);
> > >  	struct swnode *c = to_swnode(child);
> > >  
> > > -	if (!p || list_empty(&p->children) ||
> > > -	    (c && list_is_last(&c->entry, &p->children))) {
> > > -		fwnode_handle_put(child);
> > 
> > Wouldn't be better to use swnode_get() / swnode_put() instead?
> > *Yes, we might need to add some NULL checks there.
> 
> It's not newly added by me. The software_node_get_next_child() has been using
> fwnode_handle_get() / fwnode_handle_put() before. In my opinion, this should
> be fine since they do the same thing here for a swnode.

It doesn't matter who added that. But according to the point of this patch
(correct me if I am wrong) is to avoid bumping or dropping reference count for
the nodes that are *not* of swnode type. Moving away from fwnode_handle_*()
loop we make the point clear.

See the of_get_next_status_child() implementation, it does *not* use
fwnode_handle_*() at all. So, making it here to use same approach should
fix your issue, no?

> > > +	if (!p || list_empty(&p->children))
> > >  		return NULL;
> > > -	}

-- 
With Best Regards,
Andy Shevchenko