[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <20141128152512.2152EC40884@trevor.secretlab.ca>
Date: Fri, 28 Nov 2014 15:25:12 +0000
From: Grant Likely <grant.likely@...aro.org>
To: Leif Lindholm <leif.lindholm@...aro.org>
Cc: devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
linux-kernel@...r.kernel.org, mark.rutland@....com,
robh+dt@...nel.org, plagnioj@...osoft.com, ijc@...ian.org,
andrew@...n.ch, s.hauer@...gutronix.de
Subject: Re: [PATCH v3 2/3] of: add optional options parameter
to of_find_node_by_path()
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...
>
> /
> Leif
>
> From 2e1a44e539967d96366d074ae158092900e0c822 Mon Sep 17 00:00:00 2001
> From: Leif Lindholm <leif.lindholm@...aro.org>
> Date: Thu, 27 Nov 2014 09:24:31 +0000
> Subject: [PATCH] of: add optional options parameter to of_find_node_by_path()
>
> Update of_find_node_by_path():
> 1) Rename function to of_find_node_opts_by_path(), adding an optional
> pointer argument. Provide a static inline wrapper version of
> of_find_node_by_path() which calls the new function with NULL as
> the optional argument.
> 2) Ignore any part of the path beyond and including the ':' separator.
> 3) Set the new provided pointer argument to the beginning of the string
> following the ':' separator.
> 4: Add tests.
>
> Signed-off-by: Leif Lindholm <leif.lindholm@...aro.org>
> ---
> @@ -729,19 +737,24 @@ static struct device_node *__of_find_node_by_path(struct device_node *parent,
> * Returns a node pointer with refcount incremented, use
> * of_node_put() on it when done.
> */
> -struct device_node *of_find_node_by_path(const char *path)
> +struct device_node *of_find_node_opts_by_path(const char *path, const char **opts)
> {
> struct device_node *np = NULL;
> struct property *pp;
> unsigned long flags;
> + char *separator;
const char *separator.
Thanks,
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