Messages in this thread | | | Date | Fri, 4 Mar 2022 15:52:35 +0200 | From | Andy Shevchenko <> | Subject | Re: [PATCH v3 1/6] device property: Helper to match multiple connections |
| |
On Fri, Mar 04, 2022 at 03:41:33PM +0200, Sakari Ailus wrote: > On Fri, Mar 04, 2022 at 02:54:21PM +0200, Andy Shevchenko wrote: > > On Thu, Mar 03, 2022 at 02:33:46PM -0800, Bjorn Andersson wrote:
...
> > > + if (count >= matches_len && matches) { > > > + fwnode_handle_put(ep); > > > + return count; > > > + } > > > > Wouldn't be the same as > > > > if (count >= matches_len) { > > fwnode_handle_put(ep); > > break; > > } > > Don't you need to check for non-NULL matches here?
Right, this should be kept as in original patch.
> I find return above easier to read.
Okay, original code may work, so I have no strong opinion about return vs break, although I find slightly better to have a single point of return in such case.
> > ?
...
> > > + count_graph = fwnode_graph_devcon_matches(fwnode, con_id, data, match, > > > + matches, matches_len); > > > > > + matches += count_graph; > > > + matches_len -= count_graph; > > > > No, won't work when matches == NULL. > > > > Also, matches_len is expected to be 0 in that case (or at least being ignored, > > check with vsnprintf() behaviour in similar case). > > > > So, something like this, perhaps > > > > if (matches && matches_len) { > > matches += count_graph; > > matches_len -= count_graph; > > } > > Good find!
I have checked vsnprintf() and indeed, it expects to have the size is 0 when the resulting buffer pointer is NULL, and it doesn't do any additional checks.
> > > + count_ref = fwnode_devcon_matches(fwnode, con_id, data, match, > > > + matches, matches_len);
-- With Best Regards, Andy Shevchenko
| |