[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CACxGe6s=PETLBN_Y5Mk6zM677-1SrNUwAJoUK4bHdkPObBH4JQ@mail.gmail.com>
Date: Fri, 28 Nov 2014 15:33:08 +0000
From: Grant Likely <grant.likely@...aro.org>
To: Leif Lindholm <leif.lindholm@...aro.org>
Cc: "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
"linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
Mark Rutland <mark.rutland@....com>,
Rob Herring <robh+dt@...nel.org>,
Jean-Christophe PLAGNIOL-VILLARD <plagnioj@...osoft.com>,
Ian Campbell <ijc@...ian.org>, Andrew Lunn <andrew@...n.ch>,
"s.hauer@...gutronix.de" <s.hauer@...gutronix.de>
Subject: Re: 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@...aro.org> wrote:
> On Fri, 28 Nov 2014 11:34:28 +0000
> , Leif Lindholm <leif.lindholm@...aro.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.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists