mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* [PATCH v2 1/3] state: copy backend of_path string
@ 2016-09-16  6:43 Michael Olbrich
  2016-09-16  6:43 ` [PATCH v2 2/3] state: don't keep pointers to device tree nodes Michael Olbrich
                   ` (3 more replies)
  0 siblings, 4 replies; 6+ messages in thread
From: Michael Olbrich @ 2016-09-16  6:43 UTC (permalink / raw)
  To: barebox; +Cc: Michael Olbrich

Caching pointers to device tree nodes or names is not save. The barebox
internal device tree may be changed by loading a new device tree or through
fixup handlers. As a result, the string may be deleted.
Use local copies of the full path instead.

Signed-off-by: Michael Olbrich <m.olbrich@pengutronix.de>
---
Changes since v1:
- First patch split into two patches

 common/state/backend.c | 3 ++-
 common/state/state.h   | 2 +-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/common/state/backend.c b/common/state/backend.c
index 2f2e6dfd32d1..5235bb0283dd 100644
--- a/common/state/backend.c
+++ b/common/state/backend.c
@@ -164,7 +164,7 @@ int state_backend_init(struct state_backend *backend, struct device_d *dev,
 	if (ret)
 		goto out_free_format;
 
-	backend->of_path = of_path;
+	backend->of_path = xstrdup(of_path);
 
 	return 0;
 
@@ -185,4 +185,5 @@ void state_backend_free(struct state_backend *backend)
 	state_storage_free(&backend->storage);
 	if (backend->format)
 		state_format_free(backend->format);
+	free(backend->of_path);
 }
diff --git a/common/state/state.h b/common/state/state.h
index 32146ca1bbc7..f930d06195b2 100644
--- a/common/state/state.h
+++ b/common/state/state.h
@@ -87,7 +87,7 @@ struct state_backend_storage {
 struct state_backend {
 	struct state_backend_format *format;
 	struct state_backend_storage storage;
-	const char *of_path;
+	char *of_path;
 };
 
 struct state {
-- 
2.8.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [PATCH v2 2/3] state: don't keep pointers to device tree nodes
  2016-09-16  6:43 [PATCH v2 1/3] state: copy backend of_path string Michael Olbrich
@ 2016-09-16  6:43 ` Michael Olbrich
  2016-09-16  6:43 ` [PATCH v2 3/3] state: fix finding the correct parent node Michael Olbrich
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 6+ messages in thread
From: Michael Olbrich @ 2016-09-16  6:43 UTC (permalink / raw)
  To: barebox; +Cc: Michael Olbrich

Caching pointers to device tree nodes or is not save. The barebox internal
device tree may be changed by loading a new device tree or through fixup
handlers. As a result, the node may be deleted and replaced with a new one.
Keep a copy of the full path instead and resolve the node as needed.

Signed-off-by: Michael Olbrich <m.olbrich@pengutronix.de>
---
Changes since v1:
- First patch split into two patches

 common/state/state.c | 19 ++++++++++++-------
 common/state/state.h |  2 +-
 2 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/common/state/state.c b/common/state/state.c
index 9b1d4edef132..0c329cd67548 100644
--- a/common/state/state.c
+++ b/common/state/state.c
@@ -202,15 +202,19 @@ struct device_node *state_to_node(struct state *state,
 				  enum state_convert conv)
 {
 	struct device_node *child;
-	struct device_node *root;
+	struct device_node *root, *state_root;
 	int ret;
 
-	root = of_new_node(parent, state->root->name);
+	state_root = of_find_node_by_path(state->of_path);
+	if (!state_root)
+		return ERR_PTR(-ENODEV);
+
+	root = of_new_node(parent, state_root->name);
 	ret = of_property_write_u32(root, "magic", state->magic);
 	if (ret)
 		goto out;
 
-	for_each_child_of_node(state->root, child) {
+	for_each_child_of_node(state_root, child) {
 		ret = state_convert_node_variable(state, child, root, "", conv);
 		if (ret)
 			goto out;
@@ -234,7 +238,7 @@ int state_from_node(struct state *state, struct device_node *node, bool create)
 
 	if (create) {
 		conv = STATE_CONVERT_FROM_NODE_CREATE;
-		state->root = node;
+		state->of_path = xstrdup(node->full_name);
 		state->magic = magic;
 	} else {
 		conv = STATE_CONVERT_FROM_NODE;
@@ -291,7 +295,7 @@ static int of_state_fixup(struct device_node *root, void *ctx)
 	int ret;
 	phandle phandle;
 
-	node = of_find_node_by_path_from(root, state->root->full_name);
+	node = of_find_node_by_path_from(root, state->of_path);
 	if (node) {
 		/* replace existing node - it will be deleted later */
 		parent = node->parent;
@@ -299,7 +303,7 @@ static int of_state_fixup(struct device_node *root, void *ctx)
 		char *of_path, *c;
 
 		/* look for parent, remove last '/' from path */
-		of_path = xstrdup(state->root->full_name);
+		of_path = xstrdup(state->of_path);
 		c = strrchr(of_path, '/');
 		if (!c)
 			return -ENODEV;
@@ -406,6 +410,7 @@ void state_release(struct state *state)
 	list_del(&state->list);
 	unregister_device(&state->dev);
 	state_backend_free(&state->backend);
+	free(state->of_path);
 	free(state);
 }
 
@@ -545,7 +550,7 @@ struct state *state_by_node(const struct device_node *node)
 	struct state *state;
 
 	list_for_each_entry(state, &state_list, list) {
-		if (state->root == node)
+		if (!strcmp(state->of_path, node->full_name))
 			return state;
 	}
 
diff --git a/common/state/state.h b/common/state/state.h
index f930d06195b2..7b3e49512e37 100644
--- a/common/state/state.h
+++ b/common/state/state.h
@@ -94,7 +94,7 @@ struct state {
 	struct list_head list; /* Entry to enqueue on list of states */
 
 	struct device_d dev;
-	struct device_node *root;
+	char *of_path;
 	const char *name;
 	uint32_t magic;
 
-- 
2.8.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [PATCH v2 3/3] state: fix finding the correct parent node
  2016-09-16  6:43 [PATCH v2 1/3] state: copy backend of_path string Michael Olbrich
  2016-09-16  6:43 ` [PATCH v2 2/3] state: don't keep pointers to device tree nodes Michael Olbrich
@ 2016-09-16  6:43 ` Michael Olbrich
  2016-09-16  7:47 ` [PATCH v2 1/3] state: copy backend of_path string Sascha Hauer
  2016-09-16  8:12 ` Antony Pavlov
  3 siblings, 0 replies; 6+ messages in thread
From: Michael Olbrich @ 2016-09-16  6:43 UTC (permalink / raw)
  To: barebox; +Cc: Michael Olbrich

Looking for the parent node during fixup is broken. The path of the parent
node is not correctly terminated ('0' vs '\0'). Also, the new state node
should be added to the supplied device tree not the barebox device tree
used by of_find_node_by_path().

Signed-off-by: Michael Olbrich <m.olbrich@pengutronix.de>
---
 common/state/state.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/common/state/state.c b/common/state/state.c
index 0c329cd67548..075618e5bb8f 100644
--- a/common/state/state.c
+++ b/common/state/state.c
@@ -307,8 +307,8 @@ static int of_state_fixup(struct device_node *root, void *ctx)
 		c = strrchr(of_path, '/');
 		if (!c)
 			return -ENODEV;
-		*c = '0';
-		parent = of_find_node_by_path(of_path);
+		*c = '\0';
+		parent = of_find_node_by_path_from(root, of_path);
 		if (!parent)
 			parent = root;
 
-- 
2.8.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/3] state: copy backend of_path string
  2016-09-16  6:43 [PATCH v2 1/3] state: copy backend of_path string Michael Olbrich
  2016-09-16  6:43 ` [PATCH v2 2/3] state: don't keep pointers to device tree nodes Michael Olbrich
  2016-09-16  6:43 ` [PATCH v2 3/3] state: fix finding the correct parent node Michael Olbrich
@ 2016-09-16  7:47 ` Sascha Hauer
  2016-09-16  8:12 ` Antony Pavlov
  3 siblings, 0 replies; 6+ messages in thread
From: Sascha Hauer @ 2016-09-16  7:47 UTC (permalink / raw)
  To: Michael Olbrich; +Cc: barebox

On Fri, Sep 16, 2016 at 08:43:38AM +0200, Michael Olbrich wrote:
> Caching pointers to device tree nodes or names is not save. The barebox
> internal device tree may be changed by loading a new device tree or through
> fixup handlers. As a result, the string may be deleted.
> Use local copies of the full path instead.
> 
> Signed-off-by: Michael Olbrich <m.olbrich@pengutronix.de>
> ---
> Changes since v1:
> - First patch split into two patches

Applied, thanks

Sascha

> 
>  common/state/backend.c | 3 ++-
>  common/state/state.h   | 2 +-
>  2 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/common/state/backend.c b/common/state/backend.c
> index 2f2e6dfd32d1..5235bb0283dd 100644
> --- a/common/state/backend.c
> +++ b/common/state/backend.c
> @@ -164,7 +164,7 @@ int state_backend_init(struct state_backend *backend, struct device_d *dev,
>  	if (ret)
>  		goto out_free_format;
>  
> -	backend->of_path = of_path;
> +	backend->of_path = xstrdup(of_path);
>  
>  	return 0;
>  
> @@ -185,4 +185,5 @@ void state_backend_free(struct state_backend *backend)
>  	state_storage_free(&backend->storage);
>  	if (backend->format)
>  		state_format_free(backend->format);
> +	free(backend->of_path);
>  }
> diff --git a/common/state/state.h b/common/state/state.h
> index 32146ca1bbc7..f930d06195b2 100644
> --- a/common/state/state.h
> +++ b/common/state/state.h
> @@ -87,7 +87,7 @@ struct state_backend_storage {
>  struct state_backend {
>  	struct state_backend_format *format;
>  	struct state_backend_storage storage;
> -	const char *of_path;
> +	char *of_path;
>  };
>  
>  struct state {
> -- 
> 2.8.1
> 
> 
> _______________________________________________
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox
> 

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/3] state: copy backend of_path string
  2016-09-16  6:43 [PATCH v2 1/3] state: copy backend of_path string Michael Olbrich
                   ` (2 preceding siblings ...)
  2016-09-16  7:47 ` [PATCH v2 1/3] state: copy backend of_path string Sascha Hauer
@ 2016-09-16  8:12 ` Antony Pavlov
  2016-09-21  7:52   ` Sascha Hauer
  3 siblings, 1 reply; 6+ messages in thread
From: Antony Pavlov @ 2016-09-16  8:12 UTC (permalink / raw)
  To: Michael Olbrich; +Cc: barebox

On Fri, 16 Sep 2016 08:43:38 +0200
Michael Olbrich <m.olbrich@pengutronix.de> wrote:

> Caching pointers to device tree nodes or names is not save. The barebox
                                                        ^^^^ safe?
> internal device tree may be changed by loading a new device tree or through
> fixup handlers. As a result, the string may be deleted.
> Use local copies of the full path instead.
> 
> Signed-off-by: Michael Olbrich <m.olbrich@pengutronix.de>

-- 
Best regards,
  Antony Pavlov

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/3] state: copy backend of_path string
  2016-09-16  8:12 ` Antony Pavlov
@ 2016-09-21  7:52   ` Sascha Hauer
  0 siblings, 0 replies; 6+ messages in thread
From: Sascha Hauer @ 2016-09-21  7:52 UTC (permalink / raw)
  To: Antony Pavlov; +Cc: barebox, Michael Olbrich

On Fri, Sep 16, 2016 at 11:12:43AM +0300, Antony Pavlov wrote:
> On Fri, 16 Sep 2016 08:43:38 +0200
> Michael Olbrich <m.olbrich@pengutronix.de> wrote:
> 
> > Caching pointers to device tree nodes or names is not save. The barebox
>                                                         ^^^^ safe?

Fixed, thanks

Sascha

> > internal device tree may be changed by loading a new device tree or through
> > fixup handlers. As a result, the string may be deleted.
> > Use local copies of the full path instead.
> > 
> > Signed-off-by: Michael Olbrich <m.olbrich@pengutronix.de>
> 
> -- 
> Best regards,
>   Antony Pavlov
> 
> _______________________________________________
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox
> 

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-09-21  7:52 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-16  6:43 [PATCH v2 1/3] state: copy backend of_path string Michael Olbrich
2016-09-16  6:43 ` [PATCH v2 2/3] state: don't keep pointers to device tree nodes Michael Olbrich
2016-09-16  6:43 ` [PATCH v2 3/3] state: fix finding the correct parent node Michael Olbrich
2016-09-16  7:47 ` [PATCH v2 1/3] state: copy backend of_path string Sascha Hauer
2016-09-16  8:12 ` Antony Pavlov
2016-09-21  7:52   ` Sascha Hauer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox