lkml.org 
[lkml]   [2014]   [Nov]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: Re: [PATCH v3 2/3] of: add optional options parameter to of_find_node_by_path()
On Fri, Nov 28, 2014 at 3:25 PM, Grant Likely <grant.likely@linaro.org> wrote:
> On Fri, 28 Nov 2014 11:34:28 +0000
> , Leif Lindholm <leif.lindholm@linaro.org>
> wrote:
>> On Fri, Nov 28, 2014 at 12:44:03AM +0000, Grant Likely wrote:
>> > > + separator = strchr(path, ':');
>> > > + if (separator && opts)
>> > > + *opts = separator + 1;
>> > > +
>> >
>> > What about when there are no opts? Do we require the caller to make sure
>> > opts is NULL before calling the function (which sounds like a good
>> > source of bugs) or do we clear it on successful return?
>> >
>> > I think if opts is passed in, but there are no options, then it should
>> > set *opts = NULL.
>>
>> Yeah, oops.
>>
>> > There should be test cases for this also. Must set opts to NULL on
>> > successful return, and (I think) should leave opts alone on an
>> > unsuccessful search.
>>
>> I would actually argue for always nuking the opts - since that could
>> (theoretically) prevent something working by accident in spite of
>> error conditions.
>>
>> How about the below?
>
> Perfect, applied with one fixup below...

And by the way, let me say well done on this patch. It is elegantly
implemented within the framework already there.

g.


\
 
 \ /
  Last update: 2014-11-28 17:01    [W:0.167 / U:0.092 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site