On 2017-01-23 11:00, qhou wrote:
>
>
> On 2017年01月23日 16:21, Peter Rosin wrote:
>> On 2017-01-23 03:21, Qi Hou wrote:
>>> During stepping down the tree, parent node is gotten first and its refcount is
>>> increased with of_node_get() in __of_get_next_child(). Since it just being used
>>> as step node, its refcount must be decreased with of_node_put() after traversing
>>> its child nodes.
>>>
>>> Or, its refcount will never be descreased to ZERO, then it will never be freed,
>>> as well as other related memory blocks.
>>>
>>> To fix this, decrease refcount of parent with of_node_put() after
>>> __of_find_node_by_path().
>>>
>>> Signed-off-by: Qi Hou <qi.hou@xxxxxxxxxxxxx>
>>> Reviewed-by: Zhang Xiao <xiao.zhang@xxxxxxxxxxxxx>
>>> Acked-by: Bruce Ashfield <bruce.ashfield@xxxxxxxxxxxxx>
>>> ---
>>> drivers/of/base.c | 6 ++++--
>>> 1 file changed, 4 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/of/base.c b/drivers/of/base.c
>>> index d4bea3c..c72e860 100644
>>> --- a/drivers/of/base.c
>>> +++ b/drivers/of/base.c
>>> @@ -802,7 +802,7 @@ static struct device_node *__of_find_node_by_path(struct device_node *parent,
>>> */
>>> struct device_node *of_find_node_opts_by_path(const char *path, const char **opts)
>>> {
>>> - struct device_node *np = NULL;
>>> + struct device_node *np = NULL, *npp = NULL;
>> Why initialize npp? And I would declare it (and initialize it to np) inside the
>> while loop instead.
> Yes, that is a good idea. And it looks good.
>
> What about this ?
Looks good, but it is whitespace damaged and it needs to be squashed
with the prior version.
> diff --git a/drivers/of/base.c b/drivers/of/base.c
> index c72e860..091ad29 100644
> --- a/drivers/of/base.c
> +++ b/drivers/of/base.c
> @@ -802,7 +802,7 @@ static struct device_node
> *__of_find_node_by_path(struct device_node *parent,
> */
> struct device_node *of_find_node_opts_by_path(const char *path, const
> char **opts)
> {
> - struct device_node *np = NULL, *npp = NULL;
> + struct device_node *np = NULL;
> struct property *pp;
> unsigned long flags;
> const char *separator = strchr(path, ':');
> @@ -842,8 +842,8 @@ struct device_node *of_find_node_opts_by_path(const
> char *path, const char **opt
> if (!np)
> np = of_node_get(of_root);
> while (np && *path == '/') {
> + struct device_node *npp = np;
Coding standard requires a blank line here, to separate declarations from code.
With those nits fixed,
Acked-by: Peter Rosin <peda@xxxxxxxxxx>
Cheers,
peda
> path++; /* Increment past '/' delimiter */
> - npp = np;
> np = __of_find_node_by_path(npp, path);
> of_node_put(npp);
> path = strchrnul(path, '/');
> -- Best regards, --
> Qi Hou
>>
>> Cheers,
>> peda
>>
>>> struct property *pp;
>>> unsigned long flags;
>>> const char *separator = strchr(path, ':');
>>> @@ -843,7 +843,9 @@ struct device_node *of_find_node_opts_by_path(const char *path, const char **opt
>>> np = of_node_get(of_root);
>>> while (np && *path == '/') {
>>> path++; /* Increment past '/' delimiter */
>>> - np = __of_find_node_by_path(np, path);
>>> + npp = np;
>>> + np = __of_find_node_by_path(npp, path);
>>> + of_node_put(npp);
>>> path = strchrnul(path, '/');
>>> if (separator && separator < path)
>>> break;
>>>
>
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html