* [PATCH 1/3] OF: base: add sanity checks to of_new/delete_property
2013-06-21 15:15 [PATCH 0/3] OF: base: cleanup and allow empty properties Sebastian Hesselbarth
@ 2013-06-21 15:15 ` Sebastian Hesselbarth
2013-06-23 18:24 ` Sascha Hauer
2013-06-21 15:15 ` [PATCH 2/3] OF: base: use of_delete_property where applicable Sebastian Hesselbarth
` (3 subsequent siblings)
4 siblings, 1 reply; 12+ messages in thread
From: Sebastian Hesselbarth @ 2013-06-21 15:15 UTC (permalink / raw)
To: Sebastian Hesselbarth; +Cc: barebox
This adds some sanity checks to of_new_property and of_delete_property.
Also, data passed to of_new_property is only copied if non-NULL.
Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
---
Cc: barebox@lists.infradead.org
---
drivers/of/base.c | 18 +++++++++++++++++-
1 files changed, 17 insertions(+), 1 deletions(-)
diff --git a/drivers/of/base.c b/drivers/of/base.c
index 1b351ee..bcfd73a 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -1527,21 +1527,37 @@ struct property *of_new_property(struct device_node *node, const char *name,
struct property *prop;
prop = xzalloc(sizeof(*prop));
+ if (!prop)
+ return NULL;
prop->name = strdup(name);
+ if (!prop->name)
+ goto bail_out;
+
prop->length = len;
if (len) {
prop->value = xzalloc(len);
- memcpy(prop->value, data, len);
+ if (!prop->value)
+ goto bail_out;
}
+ if (len && data)
+ memcpy(prop->value, data, len);
+
list_add_tail(&prop->list, &node->properties);
return prop;
+
+bail_out:
+ of_delete_property(prop);
+ return NULL;
}
void of_delete_property(struct property *pp)
{
+ if (!pp)
+ return;
+
list_del(&pp->list);
free(pp->name);
--
1.7.2.5
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/3] OF: base: add sanity checks to of_new/delete_property
2013-06-21 15:15 ` [PATCH 1/3] OF: base: add sanity checks to of_new/delete_property Sebastian Hesselbarth
@ 2013-06-23 18:24 ` Sascha Hauer
0 siblings, 0 replies; 12+ messages in thread
From: Sascha Hauer @ 2013-06-23 18:24 UTC (permalink / raw)
To: Sebastian Hesselbarth; +Cc: barebox
On Fri, Jun 21, 2013 at 05:15:16PM +0200, Sebastian Hesselbarth wrote:
> This adds some sanity checks to of_new_property and of_delete_property.
> Also, data passed to of_new_property is only copied if non-NULL.
>
> Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
> ---
> Cc: barebox@lists.infradead.org
> ---
> drivers/of/base.c | 18 +++++++++++++++++-
> 1 files changed, 17 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/of/base.c b/drivers/of/base.c
> index 1b351ee..bcfd73a 100644
> --- a/drivers/of/base.c
> +++ b/drivers/of/base.c
> @@ -1527,21 +1527,37 @@ struct property *of_new_property(struct device_node *node, const char *name,
> struct property *prop;
>
> prop = xzalloc(sizeof(*prop));
> + if (!prop)
> + return NULL;
xzalloc always returns valid memory. It's return-mem-or-panic.
>
> prop->name = strdup(name);
> + if (!prop->name)
> + goto bail_out;
> +
> prop->length = len;
> if (len) {
> prop->value = xzalloc(len);
> - memcpy(prop->value, data, len);
> + if (!prop->value)
> + goto bail_out;
ditto.
Sascha
--
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] 12+ messages in thread
* [PATCH 2/3] OF: base: use of_delete_property where applicable
2013-06-21 15:15 [PATCH 0/3] OF: base: cleanup and allow empty properties Sebastian Hesselbarth
2013-06-21 15:15 ` [PATCH 1/3] OF: base: add sanity checks to of_new/delete_property Sebastian Hesselbarth
@ 2013-06-21 15:15 ` Sebastian Hesselbarth
2013-06-21 15:15 ` [PATCH 3/3] OF: base: add empty property value pointer Sebastian Hesselbarth
` (2 subsequent siblings)
4 siblings, 0 replies; 12+ messages in thread
From: Sebastian Hesselbarth @ 2013-06-21 15:15 UTC (permalink / raw)
To: Sebastian Hesselbarth; +Cc: barebox
This patch makes OF API use of_delete_property where applicable instead
of freeing allocated data in different places.
Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
---
Cc: barebox@lists.infradead.org
---
drivers/of/base.c | 83 ++++++++++++++++------------------------------------
1 files changed, 26 insertions(+), 57 deletions(-)
diff --git a/drivers/of/base.c b/drivers/of/base.c
index bcfd73a..7926d5d 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -899,16 +899,11 @@ int of_property_write_u8_array(struct device_node *np,
struct property *prop = of_find_property(np, propname, NULL);
u8 *val;
- if (!prop)
- prop = of_new_property(np, propname, NULL, 0);
- if (!prop)
- return -ENOMEM;
-
- free(prop->value);
+ if (prop)
+ of_delete_property(prop);
- prop->length = sizeof(*val) * sz;
- prop->value = malloc(prop->length);
- if (!prop->value)
+ prop = of_new_property(np, propname, NULL, sizeof(*val) * sz);
+ if (!prop)
return -ENOMEM;
val = prop->value;
@@ -940,18 +935,13 @@ int of_property_write_u16_array(struct device_node *np,
struct property *prop = of_find_property(np, propname, NULL);
__be16 *val;
- if (!prop)
- prop = of_new_property(np, propname, NULL, 0);
+ if (prop)
+ of_delete_property(prop);
+
+ prop = of_new_property(np, propname, NULL, sizeof(*val) * sz);
if (!prop)
return -ENOMEM;
- free(prop->value);
-
- prop->length = sizeof(*val) * sz;
- prop->value = malloc(prop->length);
- if (!prop->value)
- return -ENOMEM;
-
val = prop->value;
while (sz--)
*val++ = cpu_to_be16(*values++);
@@ -981,16 +971,11 @@ int of_property_write_u32_array(struct device_node *np,
struct property *prop = of_find_property(np, propname, NULL);
__be32 *val;
- if (!prop)
- prop = of_new_property(np, propname, NULL, 0);
- if (!prop)
- return -ENOMEM;
-
- free(prop->value);
+ if (prop)
+ of_delete_property(prop);
- prop->length = sizeof(*val) * sz;
- prop->value = malloc(prop->length);
- if (!prop->value)
+ prop = of_new_property(np, propname, NULL, sizeof(*val) * sz);
+ if (!prop)
return -ENOMEM;
val = prop->value;
@@ -1022,16 +1007,11 @@ int of_property_write_u64_array(struct device_node *np,
struct property *prop = of_find_property(np, propname, NULL);
__be32 *val;
- if (!prop)
- prop = of_new_property(np, propname, NULL, 0);
- if (!prop)
- return -ENOMEM;
-
- free(prop->value);
+ if (prop)
+ of_delete_property(prop);
- prop->length = 2 * sizeof(*val) * sz;
- prop->value = malloc(prop->length);
- if (!prop->value)
+ prop = of_new_property(np, propname, NULL, 2 * sizeof(*val) * sz);
+ if (!prop)
return -ENOMEM;
val = prop->value;
@@ -1576,26 +1556,19 @@ void of_delete_property(struct property *pp)
int of_set_property(struct device_node *np, const char *name, const void *val, int len,
int create)
{
- struct property *pp;
+ struct property *pp = of_find_property(np, name, NULL);
if (!np)
return -ENOENT;
- pp = of_find_property(np, name, NULL);
- if (pp) {
- void *data;
-
- free(pp->value);
- data = xzalloc(len);
- memcpy(data, val, len);
- pp->value = data;
- pp->length = len;
- } else {
- if (!create)
- return -ENOENT;
+ if (!pp && !create)
+ return -ENOENT;
+
+ of_delete_property(pp);
- pp = of_new_property(np, name, val, len);
- }
+ pp = of_new_property(np, name, val, len);
+ if (!pp)
+ return -ENOMEM;
return 0;
}
@@ -1819,12 +1792,8 @@ void of_free(struct device_node *node)
if (!node)
return;
- list_for_each_entry_safe(p, pt, &node->properties, list) {
- list_del(&p->list);
- free(p->name);
- free(p->value);
- free(p);
- }
+ list_for_each_entry_safe(p, pt, &node->properties, list)
+ of_delete_property(p);
list_for_each_entry_safe(n, nt, &node->children, parent_list) {
of_free(n);
--
1.7.2.5
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 3/3] OF: base: add empty property value pointer
2013-06-21 15:15 [PATCH 0/3] OF: base: cleanup and allow empty properties Sebastian Hesselbarth
2013-06-21 15:15 ` [PATCH 1/3] OF: base: add sanity checks to of_new/delete_property Sebastian Hesselbarth
2013-06-21 15:15 ` [PATCH 2/3] OF: base: use of_delete_property where applicable Sebastian Hesselbarth
@ 2013-06-21 15:15 ` Sebastian Hesselbarth
2013-06-23 18:33 ` Sascha Hauer
2013-06-24 10:49 ` [PATCH v2 1/2] OF: base: add sanity checks to of_new/delete_property Sebastian Hesselbarth
2013-06-24 10:49 ` [PATCH v2 2/2] OF: base: use of_delete_property where applicable Sebastian Hesselbarth
4 siblings, 1 reply; 12+ messages in thread
From: Sebastian Hesselbarth @ 2013-06-21 15:15 UTC (permalink / raw)
To: Sebastian Hesselbarth; +Cc: barebox
Since property values can be empty, we need property value pointer to
be non-NULL to distinguish those properties from non-existing properties.
This adds a static u32 to which empty properties set their value pointer.
Also, the value memory is only freed, if property length is not zero.
Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
---
Cc: barebox@lists.infradead.org
---
drivers/of/base.c | 6 +++++-
1 files changed, 5 insertions(+), 1 deletions(-)
diff --git a/drivers/of/base.c b/drivers/of/base.c
index 7926d5d..a100a17 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -1501,6 +1501,8 @@ struct device_node *of_new_node(struct device_node *parent, const char *name)
return node;
}
+static u32 empty_prop_value;
+
struct property *of_new_property(struct device_node *node, const char *name,
const void *data, int len)
{
@@ -1515,6 +1517,7 @@ struct property *of_new_property(struct device_node *node, const char *name,
goto bail_out;
prop->length = len;
+ prop->value = &empty_prop_value;
if (len) {
prop->value = xzalloc(len);
if (!prop->value)
@@ -1541,7 +1544,8 @@ void of_delete_property(struct property *pp)
list_del(&pp->list);
free(pp->name);
- free(pp->value);
+ if (pp->length)
+ free(pp->value);
free(pp);
}
--
1.7.2.5
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 3/3] OF: base: add empty property value pointer
2013-06-21 15:15 ` [PATCH 3/3] OF: base: add empty property value pointer Sebastian Hesselbarth
@ 2013-06-23 18:33 ` Sascha Hauer
2013-06-23 20:11 ` Sebastian Hesselbarth
0 siblings, 1 reply; 12+ messages in thread
From: Sascha Hauer @ 2013-06-23 18:33 UTC (permalink / raw)
To: Sebastian Hesselbarth; +Cc: barebox
On Fri, Jun 21, 2013 at 05:15:18PM +0200, Sebastian Hesselbarth wrote:
> Since property values can be empty, we need property value pointer to
> be non-NULL to distinguish those properties from non-existing properties.
> This adds a static u32 to which empty properties set their value pointer.
> Also, the value memory is only freed, if property length is not zero.
>
> Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
> ---
> Cc: barebox@lists.infradead.org
> ---
> drivers/of/base.c | 6 +++++-
> 1 files changed, 5 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/of/base.c b/drivers/of/base.c
> index 7926d5d..a100a17 100644
> --- a/drivers/of/base.c
> +++ b/drivers/of/base.c
> @@ -1501,6 +1501,8 @@ struct device_node *of_new_node(struct device_node *parent, const char *name)
> return node;
> }
>
> +static u32 empty_prop_value;
> +
> struct property *of_new_property(struct device_node *node, const char *name,
> const void *data, int len)
> {
> @@ -1515,6 +1517,7 @@ struct property *of_new_property(struct device_node *node, const char *name,
> goto bail_out;
>
> prop->length = len;
> + prop->value = &empty_prop_value;
This at least breaks of_set_property() and of_free(). Both unconditionally
do a free(pp->value).
Why do we need this anyway? We can always call of_find_property() to see
if a property exists.
Sascha
--
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] 12+ messages in thread
* Re: [PATCH 3/3] OF: base: add empty property value pointer
2013-06-23 18:33 ` Sascha Hauer
@ 2013-06-23 20:11 ` Sebastian Hesselbarth
2013-06-23 20:26 ` Sascha Hauer
0 siblings, 1 reply; 12+ messages in thread
From: Sebastian Hesselbarth @ 2013-06-23 20:11 UTC (permalink / raw)
To: Sascha Hauer; +Cc: barebox
On 06/23/2013 08:33 PM, Sascha Hauer wrote:
> On Fri, Jun 21, 2013 at 05:15:18PM +0200, Sebastian Hesselbarth wrote:
>> Since property values can be empty, we need property value pointer to
>> be non-NULL to distinguish those properties from non-existing properties.
>> This adds a static u32 to which empty properties set their value pointer.
>> Also, the value memory is only freed, if property length is not zero.
>>
>> Signed-off-by: Sebastian Hesselbarth<sebastian.hesselbarth@gmail.com>
>> ---
>> Cc: barebox@lists.infradead.org
>> ---
>> drivers/of/base.c | 6 +++++-
>> 1 files changed, 5 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/of/base.c b/drivers/of/base.c
>> index 7926d5d..a100a17 100644
>> --- a/drivers/of/base.c
>> +++ b/drivers/of/base.c
>> @@ -1501,6 +1501,8 @@ struct device_node *of_new_node(struct device_node *parent, const char *name)
>> return node;
>> }
>>
>> +static u32 empty_prop_value;
>> +
>> struct property *of_new_property(struct device_node *node, const char *name,
>> const void *data, int len)
>> {
>> @@ -1515,6 +1517,7 @@ struct property *of_new_property(struct device_node *node, const char *name,
>> goto bail_out;
>>
>> prop->length = len;
>> + prop->value =&empty_prop_value;
>
> This at least breaks of_set_property() and of_free(). Both unconditionally
> do a free(pp->value).
Sascha,
this is patch 3/3, the two functions above are taken care of patch 2/3.
> Why do we need this anyway? We can always call of_find_property() to see
> if a property exists.
Actually, I was preparing to import drivers/of/address.c from linux.
of_translate_one uses of_get_property on "ranges". This returns
the property's value pointer instead of the property itself, which
is NULL for an empty ranges property. Now that you point it out,
using of_find_property is also an option.
Nevertheless, patches 1 (with your comments applied) and 2 seem
sensible to me. Patch 3 is an option to keep it in sync with Linux
OF API behavior but is optional.
Sebastian
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 3/3] OF: base: add empty property value pointer
2013-06-23 20:11 ` Sebastian Hesselbarth
@ 2013-06-23 20:26 ` Sascha Hauer
2013-06-23 20:29 ` Sebastian Hesselbarth
0 siblings, 1 reply; 12+ messages in thread
From: Sascha Hauer @ 2013-06-23 20:26 UTC (permalink / raw)
To: Sebastian Hesselbarth; +Cc: barebox
On Sun, Jun 23, 2013 at 10:11:39PM +0200, Sebastian Hesselbarth wrote:
> On 06/23/2013 08:33 PM, Sascha Hauer wrote:
> >On Fri, Jun 21, 2013 at 05:15:18PM +0200, Sebastian Hesselbarth wrote:
> >>Since property values can be empty, we need property value pointer to
> >>be non-NULL to distinguish those properties from non-existing properties.
> >>This adds a static u32 to which empty properties set their value pointer.
> >>Also, the value memory is only freed, if property length is not zero.
> >>
> >>Signed-off-by: Sebastian Hesselbarth<sebastian.hesselbarth@gmail.com>
> >>---
> >>Cc: barebox@lists.infradead.org
> >>---
> >> drivers/of/base.c | 6 +++++-
> >> 1 files changed, 5 insertions(+), 1 deletions(-)
> >>
> >>diff --git a/drivers/of/base.c b/drivers/of/base.c
> >>index 7926d5d..a100a17 100644
> >>--- a/drivers/of/base.c
> >>+++ b/drivers/of/base.c
> >>@@ -1501,6 +1501,8 @@ struct device_node *of_new_node(struct device_node *parent, const char *name)
> >> return node;
> >> }
> >>
> >>+static u32 empty_prop_value;
> >>+
> >> struct property *of_new_property(struct device_node *node, const char *name,
> >> const void *data, int len)
> >> {
> >>@@ -1515,6 +1517,7 @@ struct property *of_new_property(struct device_node *node, const char *name,
> >> goto bail_out;
> >>
> >> prop->length = len;
> >>+ prop->value =&empty_prop_value;
> >
> >This at least breaks of_set_property() and of_free(). Both unconditionally
> >do a free(pp->value).
>
> Sascha,
>
> this is patch 3/3, the two functions above are taken care of patch 2/3.
Ah, sorry, I missed that.
>
> >Why do we need this anyway? We can always call of_find_property() to see
> >if a property exists.
>
> Actually, I was preparing to import drivers/of/address.c from linux.
> of_translate_one uses of_get_property on "ranges". This returns
> the property's value pointer instead of the property itself, which
> is NULL for an empty ranges property. Now that you point it out,
> using of_find_property is also an option.
>
> Nevertheless, patches 1 (with your comments applied) and 2 seem
> sensible to me. Patch 3 is an option to keep it in sync with Linux
> OF API behavior but is optional.
How about allocating zero length property values with malloc(0)? This
makes them less special.
Sascha
--
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] 12+ messages in thread
* Re: [PATCH 3/3] OF: base: add empty property value pointer
2013-06-23 20:26 ` Sascha Hauer
@ 2013-06-23 20:29 ` Sebastian Hesselbarth
0 siblings, 0 replies; 12+ messages in thread
From: Sebastian Hesselbarth @ 2013-06-23 20:29 UTC (permalink / raw)
To: Sascha Hauer; +Cc: barebox
On 06/23/2013 10:26 PM, Sascha Hauer wrote:
> On Sun, Jun 23, 2013 at 10:11:39PM +0200, Sebastian Hesselbarth wrote:
>> On 06/23/2013 08:33 PM, Sascha Hauer wrote:
>>> Why do we need this anyway? We can always call of_find_property() to see
>>> if a property exists.
>>
>> Actually, I was preparing to import drivers/of/address.c from linux.
>> of_translate_one uses of_get_property on "ranges". This returns
>> the property's value pointer instead of the property itself, which
>> is NULL for an empty ranges property. Now that you point it out,
>> using of_find_property is also an option.
>>
>> Nevertheless, patches 1 (with your comments applied) and 2 seem
>> sensible to me. Patch 3 is an option to keep it in sync with Linux
>> OF API behavior but is optional.
>
> How about allocating zero length property values with malloc(0)? This
> makes them less special.
Agree, I will update the patch set and send a v2 hopefully soon.
Sebastian
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 1/2] OF: base: add sanity checks to of_new/delete_property
2013-06-21 15:15 [PATCH 0/3] OF: base: cleanup and allow empty properties Sebastian Hesselbarth
` (2 preceding siblings ...)
2013-06-21 15:15 ` [PATCH 3/3] OF: base: add empty property value pointer Sebastian Hesselbarth
@ 2013-06-24 10:49 ` Sebastian Hesselbarth
2013-06-24 19:40 ` Sascha Hauer
2013-06-24 10:49 ` [PATCH v2 2/2] OF: base: use of_delete_property where applicable Sebastian Hesselbarth
4 siblings, 1 reply; 12+ messages in thread
From: Sebastian Hesselbarth @ 2013-06-24 10:49 UTC (permalink / raw)
To: Sebastian Hesselbarth; +Cc: barebox
This adds some sanity checks to of_new_property and of_delete_property.
Also, value pointer is always allocated even with zero length to allow
empty properties to be distinguished from non-existing properties.
Finally, data passed to of_new_property is only copied if non-NULL.
Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
---
Note: Patch 3 of the former patch set has been dropped in favour of zero
length allocation here.
Changelog:
v1->v2:
- remove unneccesary NULL checks after xzalloc (Suggested by Sascha Hauer)
- allocate zero length value pointer (Suggested by Sascha Hauer)
Cc: barebox@lists.infradead.org
---
drivers/of/base.c | 15 +++++++++++----
1 files changed, 11 insertions(+), 4 deletions(-)
diff --git a/drivers/of/base.c b/drivers/of/base.c
index 1b351ee..e65cf85 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -1527,13 +1527,17 @@ struct property *of_new_property(struct device_node *node, const char *name,
struct property *prop;
prop = xzalloc(sizeof(*prop));
-
prop->name = strdup(name);
+ if (!prop->name) {
+ free(prop);
+ return NULL;
+ }
+
prop->length = len;
- if (len) {
- prop->value = xzalloc(len);
+ prop->value = xzalloc(len);
+
+ if (data)
memcpy(prop->value, data, len);
- }
list_add_tail(&prop->list, &node->properties);
@@ -1542,6 +1546,9 @@ struct property *of_new_property(struct device_node *node, const char *name,
void of_delete_property(struct property *pp)
{
+ if (!pp)
+ return;
+
list_del(&pp->list);
free(pp->name);
--
1.7.2.5
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/2] OF: base: add sanity checks to of_new/delete_property
2013-06-24 10:49 ` [PATCH v2 1/2] OF: base: add sanity checks to of_new/delete_property Sebastian Hesselbarth
@ 2013-06-24 19:40 ` Sascha Hauer
0 siblings, 0 replies; 12+ messages in thread
From: Sascha Hauer @ 2013-06-24 19:40 UTC (permalink / raw)
To: Sebastian Hesselbarth; +Cc: barebox
On Mon, Jun 24, 2013 at 12:49:20PM +0200, Sebastian Hesselbarth wrote:
> This adds some sanity checks to of_new_property and of_delete_property.
> Also, value pointer is always allocated even with zero length to allow
> empty properties to be distinguished from non-existing properties.
> Finally, data passed to of_new_property is only copied if non-NULL.
>
> Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
Applied both.
Thanks
Sascha
> ---
> Note: Patch 3 of the former patch set has been dropped in favour of zero
> length allocation here.
>
> Changelog:
> v1->v2:
> - remove unneccesary NULL checks after xzalloc (Suggested by Sascha Hauer)
> - allocate zero length value pointer (Suggested by Sascha Hauer)
>
> Cc: barebox@lists.infradead.org
> ---
> drivers/of/base.c | 15 +++++++++++----
> 1 files changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/of/base.c b/drivers/of/base.c
> index 1b351ee..e65cf85 100644
> --- a/drivers/of/base.c
> +++ b/drivers/of/base.c
> @@ -1527,13 +1527,17 @@ struct property *of_new_property(struct device_node *node, const char *name,
> struct property *prop;
>
> prop = xzalloc(sizeof(*prop));
> -
> prop->name = strdup(name);
> + if (!prop->name) {
> + free(prop);
> + return NULL;
> + }
> +
> prop->length = len;
> - if (len) {
> - prop->value = xzalloc(len);
> + prop->value = xzalloc(len);
> +
> + if (data)
> memcpy(prop->value, data, len);
> - }
>
> list_add_tail(&prop->list, &node->properties);
>
> @@ -1542,6 +1546,9 @@ struct property *of_new_property(struct device_node *node, const char *name,
>
> void of_delete_property(struct property *pp)
> {
> + if (!pp)
> + return;
> +
> list_del(&pp->list);
>
> free(pp->name);
> --
> 1.7.2.5
>
>
> _______________________________________________
> 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] 12+ messages in thread
* [PATCH v2 2/2] OF: base: use of_delete_property where applicable
2013-06-21 15:15 [PATCH 0/3] OF: base: cleanup and allow empty properties Sebastian Hesselbarth
` (3 preceding siblings ...)
2013-06-24 10:49 ` [PATCH v2 1/2] OF: base: add sanity checks to of_new/delete_property Sebastian Hesselbarth
@ 2013-06-24 10:49 ` Sebastian Hesselbarth
4 siblings, 0 replies; 12+ messages in thread
From: Sebastian Hesselbarth @ 2013-06-24 10:49 UTC (permalink / raw)
To: Sebastian Hesselbarth; +Cc: barebox
This patch makes OF API use of_delete_property where applicable instead
of freeing allocated data in different places.
Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
---
Changelog:
v1->v2:
- remove offending whitespace lines
Cc: barebox@lists.infradead.org
---
drivers/of/base.c | 81 ++++++++++++++++------------------------------------
1 files changed, 25 insertions(+), 56 deletions(-)
diff --git a/drivers/of/base.c b/drivers/of/base.c
index e65cf85..63ff647 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -899,16 +899,11 @@ int of_property_write_u8_array(struct device_node *np,
struct property *prop = of_find_property(np, propname, NULL);
u8 *val;
- if (!prop)
- prop = of_new_property(np, propname, NULL, 0);
- if (!prop)
- return -ENOMEM;
-
- free(prop->value);
+ if (prop)
+ of_delete_property(prop);
- prop->length = sizeof(*val) * sz;
- prop->value = malloc(prop->length);
- if (!prop->value)
+ prop = of_new_property(np, propname, NULL, sizeof(*val) * sz);
+ if (!prop)
return -ENOMEM;
val = prop->value;
@@ -940,16 +935,11 @@ int of_property_write_u16_array(struct device_node *np,
struct property *prop = of_find_property(np, propname, NULL);
__be16 *val;
- if (!prop)
- prop = of_new_property(np, propname, NULL, 0);
- if (!prop)
- return -ENOMEM;
-
- free(prop->value);
+ if (prop)
+ of_delete_property(prop);
- prop->length = sizeof(*val) * sz;
- prop->value = malloc(prop->length);
- if (!prop->value)
+ prop = of_new_property(np, propname, NULL, sizeof(*val) * sz);
+ if (!prop)
return -ENOMEM;
val = prop->value;
@@ -981,16 +971,11 @@ int of_property_write_u32_array(struct device_node *np,
struct property *prop = of_find_property(np, propname, NULL);
__be32 *val;
- if (!prop)
- prop = of_new_property(np, propname, NULL, 0);
- if (!prop)
- return -ENOMEM;
+ if (prop)
+ of_delete_property(prop);
- free(prop->value);
-
- prop->length = sizeof(*val) * sz;
- prop->value = malloc(prop->length);
- if (!prop->value)
+ prop = of_new_property(np, propname, NULL, sizeof(*val) * sz);
+ if (!prop)
return -ENOMEM;
val = prop->value;
@@ -1022,16 +1007,11 @@ int of_property_write_u64_array(struct device_node *np,
struct property *prop = of_find_property(np, propname, NULL);
__be32 *val;
- if (!prop)
- prop = of_new_property(np, propname, NULL, 0);
- if (!prop)
- return -ENOMEM;
+ if (prop)
+ of_delete_property(prop);
- free(prop->value);
-
- prop->length = 2 * sizeof(*val) * sz;
- prop->value = malloc(prop->length);
- if (!prop->value)
+ prop = of_new_property(np, propname, NULL, 2 * sizeof(*val) * sz);
+ if (!prop)
return -ENOMEM;
val = prop->value;
@@ -1567,26 +1547,19 @@ void of_delete_property(struct property *pp)
int of_set_property(struct device_node *np, const char *name, const void *val, int len,
int create)
{
- struct property *pp;
+ struct property *pp = of_find_property(np, name, NULL);
if (!np)
return -ENOENT;
- pp = of_find_property(np, name, NULL);
- if (pp) {
- void *data;
+ if (!pp && !create)
+ return -ENOENT;
- free(pp->value);
- data = xzalloc(len);
- memcpy(data, val, len);
- pp->value = data;
- pp->length = len;
- } else {
- if (!create)
- return -ENOENT;
+ of_delete_property(pp);
- pp = of_new_property(np, name, val, len);
- }
+ pp = of_new_property(np, name, val, len);
+ if (!pp)
+ return -ENOMEM;
return 0;
}
@@ -1810,12 +1783,8 @@ void of_free(struct device_node *node)
if (!node)
return;
- list_for_each_entry_safe(p, pt, &node->properties, list) {
- list_del(&p->list);
- free(p->name);
- free(p->value);
- free(p);
- }
+ list_for_each_entry_safe(p, pt, &node->properties, list)
+ of_delete_property(p);
list_for_each_entry_safe(n, nt, &node->children, parent_list) {
of_free(n);
--
1.7.2.5
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 12+ messages in thread