Re: [PATCH v3 2/3] of: add optional options parameter to of_find_node_by_path()

[Grant Likely]

On Fri, 28 Nov 2014 11:34:28 +0000
, Leif Lindholm <leif.lindholm@xxxxxxxxxx>
 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@xxxxxxxxxx>
> 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@xxxxxxxxxx>
> ---
> @@ -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@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/