* [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