Re: [PATCH 1/3] of: dynamic: add of_property_alloc() and of_property_free()
On 6/2/22 07:06, Rob Herring wrote:
> On Wed, Jun 1, 2022 at 5:31 PM Tyrel Datwyler wrote:
>>
>> On 5/5/22 12:37, Rob Herring wrote:
>>> On Wed, May 04, 2022 at 05:40:31PM +0200, Clément Léger wrote:
Add function which allows to dynamically allocate and free properties.
Use this function internally for all code that used the same logic
(mainly __of_prop_dup()).
Signed-off-by: Clément Léger
---
drivers/of/dynamic.c | 101 ++-
include/linux/of.h | 16 +++
2 files changed, 88 insertions(+), 29 deletions(-)
diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c
index cd3821a6444f..e8700e509d2e 100644
--- a/drivers/of/dynamic.c
+++ b/drivers/of/dynamic.c
@@ -313,9 +313,7 @@ static void property_list_free(struct property
*prop_list)
for (prop = prop_list; prop != NULL; prop = next) {
next = prop->next;
-kfree(prop->name);
-kfree(prop->value);
-kfree(prop);
+of_property_free(prop);
}
}
@@ -367,48 +365,95 @@ void of_node_release(struct kobject *kobj)
}
/**
- * __of_prop_dup - Copy a property dynamically.
- * @prop: Property to copy
+ * of_property_free - Free a property allocated dynamically.
+ * @prop: Property to be freed
+ */
+void of_property_free(const struct property *prop)
+{
+kfree(prop->value);
+kfree(prop->name);
+kfree(prop);
+}
+EXPORT_SYMBOL(of_property_free);
+
+/**
+ * of_property_alloc - Allocate a property dynamically.
+ * @name: Name of the new property
+ * @value: Value that will be copied into the new property value
+ * @value_len: length of @value to be copied into the new property
value
+ * @len:Length of new property value, must be greater than @value_len
>>>
>>> What's the usecase for the lengths being different? That doesn't seem
>>> like a common case, so perhaps handle it with a NULL value and
>>> non-zero length. Then the caller has to deal with populating
>>> prop->value.
>>>
* @allocflags: Allocation flags (typically pass GFP_KERNEL)
*
- * Copy a property by dynamically allocating the memory of both the
+ * Create a property by dynamically allocating the memory of both the
* property structure and the property name & contents. The property's
* flags have the OF_DYNAMIC bit set so that we can differentiate between
* dynamically allocated properties and not.
*
* Return: The newly allocated property or NULL on out of memory error.
*/
-struct property *__of_prop_dup(const struct property *prop, gfp_t
allocflags)
+struct property *of_property_alloc(const char *name, const void *value,
+ int value_len, int len, gfp_t allocflags)
{
-struct property *new;
+int alloc_len = len;
+struct property *prop;
+
+if (len < value_len)
+return NULL;
-new = kzalloc(sizeof(*new), allocflags);
-if (!new)
+prop = kzalloc(sizeof(*prop), allocflags);
+if (!prop)
return NULL;
+prop->name = kstrdup(name, allocflags);
+if (!prop->name)
+goto out_err;
+
/*
- * NOTE: There is no check for zero length value.
- * In case of a boolean property, this will allocate a value
- * of zero bytes. We do this to work around the use
- * of of_get_property() calls on boolean values.
+ * Even if the property has no value, it must be set to a
+ * non-null value since of_get_property() is used to check
+ * some values that might or not have a values (ranges for
+ * instance). Moreover, when the node is released, prop->value
+ * is kfreed so the memory must come from kmalloc.
>>>
>>> Allowing for NULL value didn't turn out well...
>>>
>>> We know that we can do the kfree because OF_DYNAMIC is set IIRC...
>>>
>>> If we do 1 allocation for prop and value, then we can test
>>> for "prop->value == prop + 1" to determine if we need to free or not.
>>
>> If its a single allocation do we even need a test? Doesn't kfree(prop) take
>> care
>> of the property and the trailing memory allocated for the value?
>
> Yes, it does when it's a single alloc, but it's testing for when
> prop->value is not a single allocation because we could have either.
>
Ok, that is the part I was missing. Thanks for the clarification.
-Tyrel
Re: [PATCH 1/3] of: dynamic: add of_property_alloc() and of_property_free()
On Wed, Jun 1, 2022 at 5:31 PM Tyrel Datwyler wrote:
>
> On 5/5/22 12:37, Rob Herring wrote:
> > On Wed, May 04, 2022 at 05:40:31PM +0200, Clément Léger wrote:
> >> Add function which allows to dynamically allocate and free properties.
> >> Use this function internally for all code that used the same logic
> >> (mainly __of_prop_dup()).
> >>
> >> Signed-off-by: Clément Léger
> >> ---
> >> drivers/of/dynamic.c | 101 ++-
> >> include/linux/of.h | 16 +++
> >> 2 files changed, 88 insertions(+), 29 deletions(-)
> >>
> >> diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c
> >> index cd3821a6444f..e8700e509d2e 100644
> >> --- a/drivers/of/dynamic.c
> >> +++ b/drivers/of/dynamic.c
> >> @@ -313,9 +313,7 @@ static void property_list_free(struct property
> >> *prop_list)
> >>
> >> for (prop = prop_list; prop != NULL; prop = next) {
> >> next = prop->next;
> >> -kfree(prop->name);
> >> -kfree(prop->value);
> >> -kfree(prop);
> >> +of_property_free(prop);
> >> }
> >> }
> >>
> >> @@ -367,48 +365,95 @@ void of_node_release(struct kobject *kobj)
> >> }
> >>
> >> /**
> >> - * __of_prop_dup - Copy a property dynamically.
> >> - * @prop: Property to copy
> >> + * of_property_free - Free a property allocated dynamically.
> >> + * @prop: Property to be freed
> >> + */
> >> +void of_property_free(const struct property *prop)
> >> +{
> >> +kfree(prop->value);
> >> +kfree(prop->name);
> >> +kfree(prop);
> >> +}
> >> +EXPORT_SYMBOL(of_property_free);
> >> +
> >> +/**
> >> + * of_property_alloc - Allocate a property dynamically.
> >> + * @name: Name of the new property
> >> + * @value: Value that will be copied into the new property value
> >> + * @value_len: length of @value to be copied into the new property
> >> value
> >> + * @len:Length of new property value, must be greater than @value_len
> >
> > What's the usecase for the lengths being different? That doesn't seem
> > like a common case, so perhaps handle it with a NULL value and
> > non-zero length. Then the caller has to deal with populating
> > prop->value.
> >
> >> * @allocflags: Allocation flags (typically pass GFP_KERNEL)
> >> *
> >> - * Copy a property by dynamically allocating the memory of both the
> >> + * Create a property by dynamically allocating the memory of both the
> >> * property structure and the property name & contents. The property's
> >> * flags have the OF_DYNAMIC bit set so that we can differentiate between
> >> * dynamically allocated properties and not.
> >> *
> >> * Return: The newly allocated property or NULL on out of memory error.
> >> */
> >> -struct property *__of_prop_dup(const struct property *prop, gfp_t
> >> allocflags)
> >> +struct property *of_property_alloc(const char *name, const void *value,
> >> + int value_len, int len, gfp_t allocflags)
> >> {
> >> -struct property *new;
> >> +int alloc_len = len;
> >> +struct property *prop;
> >> +
> >> +if (len < value_len)
> >> +return NULL;
> >>
> >> -new = kzalloc(sizeof(*new), allocflags);
> >> -if (!new)
> >> +prop = kzalloc(sizeof(*prop), allocflags);
> >> +if (!prop)
> >> return NULL;
> >>
> >> +prop->name = kstrdup(name, allocflags);
> >> +if (!prop->name)
> >> +goto out_err;
> >> +
> >> /*
> >> - * NOTE: There is no check for zero length value.
> >> - * In case of a boolean property, this will allocate a value
> >> - * of zero bytes. We do this to work around the use
> >> - * of of_get_property() calls on boolean values.
> >> + * Even if the property has no value, it must be set to a
> >> + * non-null value since of_get_property() is used to check
> >> + * some values that might or not have a values (ranges for
> >> + * instance). Moreover, when the node is released, prop->value
> >> + * is kfreed so the memory must come from kmalloc.
> >
> > Allowing for NULL value didn't turn out well...
> >
> > We know that we can do the kfree because OF_DYNAMIC is set IIRC...
> >
> > If we do 1 allocation for prop and value, then we can test
> > for "prop->value == prop + 1" to determine if we need to free or not.
>
> If its a single allocation do we even need a test? Doesn't kfree(prop) take
> care
> of the property and the trailing memory allocated for the value?
Yes, it does when it's a single alloc, but it's testing for when
prop->value is not a single allocation because we could have either.
Rob
Re: [PATCH 1/3] of: dynamic: add of_property_alloc() and of_property_free()
On 5/5/22 12:37, Rob Herring wrote:
> On Wed, May 04, 2022 at 05:40:31PM +0200, Clément Léger wrote:
>> Add function which allows to dynamically allocate and free properties.
>> Use this function internally for all code that used the same logic
>> (mainly __of_prop_dup()).
>>
>> Signed-off-by: Clément Léger
>> ---
>> drivers/of/dynamic.c | 101 ++-
>> include/linux/of.h | 16 +++
>> 2 files changed, 88 insertions(+), 29 deletions(-)
>>
>> diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c
>> index cd3821a6444f..e8700e509d2e 100644
>> --- a/drivers/of/dynamic.c
>> +++ b/drivers/of/dynamic.c
>> @@ -313,9 +313,7 @@ static void property_list_free(struct property
>> *prop_list)
>>
>> for (prop = prop_list; prop != NULL; prop = next) {
>> next = prop->next;
>> -kfree(prop->name);
>> -kfree(prop->value);
>> -kfree(prop);
>> +of_property_free(prop);
>> }
>> }
>>
>> @@ -367,48 +365,95 @@ void of_node_release(struct kobject *kobj)
>> }
>>
>> /**
>> - * __of_prop_dup - Copy a property dynamically.
>> - * @prop: Property to copy
>> + * of_property_free - Free a property allocated dynamically.
>> + * @prop: Property to be freed
>> + */
>> +void of_property_free(const struct property *prop)
>> +{
>> +kfree(prop->value);
>> +kfree(prop->name);
>> +kfree(prop);
>> +}
>> +EXPORT_SYMBOL(of_property_free);
>> +
>> +/**
>> + * of_property_alloc - Allocate a property dynamically.
>> + * @name: Name of the new property
>> + * @value: Value that will be copied into the new property value
>> + * @value_len: length of @value to be copied into the new property
>> value
>> + * @len:Length of new property value, must be greater than @value_len
>
> What's the usecase for the lengths being different? That doesn't seem
> like a common case, so perhaps handle it with a NULL value and
> non-zero length. Then the caller has to deal with populating
> prop->value.
>
>> * @allocflags: Allocation flags (typically pass GFP_KERNEL)
>> *
>> - * Copy a property by dynamically allocating the memory of both the
>> + * Create a property by dynamically allocating the memory of both the
>> * property structure and the property name & contents. The property's
>> * flags have the OF_DYNAMIC bit set so that we can differentiate between
>> * dynamically allocated properties and not.
>> *
>> * Return: The newly allocated property or NULL on out of memory error.
>> */
>> -struct property *__of_prop_dup(const struct property *prop, gfp_t
>> allocflags)
>> +struct property *of_property_alloc(const char *name, const void *value,
>> + int value_len, int len, gfp_t allocflags)
>> {
>> -struct property *new;
>> +int alloc_len = len;
>> +struct property *prop;
>> +
>> +if (len < value_len)
>> +return NULL;
>>
>> -new = kzalloc(sizeof(*new), allocflags);
>> -if (!new)
>> +prop = kzalloc(sizeof(*prop), allocflags);
>> +if (!prop)
>> return NULL;
>>
>> +prop->name = kstrdup(name, allocflags);
>> +if (!prop->name)
>> +goto out_err;
>> +
>> /*
>> - * NOTE: There is no check for zero length value.
>> - * In case of a boolean property, this will allocate a value
>> - * of zero bytes. We do this to work around the use
>> - * of of_get_property() calls on boolean values.
>> + * Even if the property has no value, it must be set to a
>> + * non-null value since of_get_property() is used to check
>> + * some values that might or not have a values (ranges for
>> + * instance). Moreover, when the node is released, prop->value
>> + * is kfreed so the memory must come from kmalloc.
>
> Allowing for NULL value didn't turn out well...
>
> We know that we can do the kfree because OF_DYNAMIC is set IIRC...
>
> If we do 1 allocation for prop and value, then we can test
> for "prop->value == prop + 1" to determine if we need to free or not.
If its a single allocation do we even need a test? Doesn't kfree(prop) take care
of the property and the trailing memory allocated for the value?
-Tyrel
>
>> */
>> -new->name = kstrdup(prop->name, allocflags);
>> -new->value = kmemdup(prop->value, prop->length, allocflags);
>> -new->length = prop->length;
>> -if (!new->name || !new->value)
>> -goto err_free;
>> +if (!alloc_len)
>> +alloc_len = 1;
>>
>> -/* mark the property as dynamic */
>> -of_property_set_flag(new, OF_DYNAMIC);
>> +prop->value = kzalloc(alloc_len, allocflags);
>> +if (!prop->value)
>> +goto out_err;
>>
>> -return new;
>> +if (value)
>> +memcpy(prop->value, value, value_len);
>> +
>> +prop->length = len;
>> +of_property_set_flag(prop, OF_DYNAMIC);
>> +
>> +return prop;
>> +
>> +out_err:
>> +of_property_free(prop);
>>
Re: [PATCH 1/3] of: dynamic: add of_property_alloc() and of_property_free()
On Wed, May 04, 2022 at 05:40:31PM +0200, Clément Léger wrote:
> Add function which allows to dynamically allocate and free properties.
> Use this function internally for all code that used the same logic
> (mainly __of_prop_dup()).
>
> Signed-off-by: Clément Léger
> ---
> drivers/of/dynamic.c | 101 ++-
> include/linux/of.h | 16 +++
> 2 files changed, 88 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c
> index cd3821a6444f..e8700e509d2e 100644
> --- a/drivers/of/dynamic.c
> +++ b/drivers/of/dynamic.c
> @@ -313,9 +313,7 @@ static void property_list_free(struct property *prop_list)
>
> for (prop = prop_list; prop != NULL; prop = next) {
> next = prop->next;
> - kfree(prop->name);
> - kfree(prop->value);
> - kfree(prop);
> + of_property_free(prop);
> }
> }
>
> @@ -367,48 +365,95 @@ void of_node_release(struct kobject *kobj)
> }
>
> /**
> - * __of_prop_dup - Copy a property dynamically.
> - * @prop:Property to copy
> + * of_property_free - Free a property allocated dynamically.
> + * @prop:Property to be freed
> + */
> +void of_property_free(const struct property *prop)
> +{
> + kfree(prop->value);
> + kfree(prop->name);
> + kfree(prop);
> +}
> +EXPORT_SYMBOL(of_property_free);
> +
> +/**
> + * of_property_alloc - Allocate a property dynamically.
> + * @name:Name of the new property
> + * @value: Value that will be copied into the new property value
> + * @value_len: length of @value to be copied into the new property
> value
> + * @len: Length of new property value, must be greater than @value_len
What's the usecase for the lengths being different? That doesn't seem
like a common case, so perhaps handle it with a NULL value and
non-zero length. Then the caller has to deal with populating
prop->value.
> * @allocflags: Allocation flags (typically pass GFP_KERNEL)
> *
> - * Copy a property by dynamically allocating the memory of both the
> + * Create a property by dynamically allocating the memory of both the
> * property structure and the property name & contents. The property's
> * flags have the OF_DYNAMIC bit set so that we can differentiate between
> * dynamically allocated properties and not.
> *
> * Return: The newly allocated property or NULL on out of memory error.
> */
> -struct property *__of_prop_dup(const struct property *prop, gfp_t allocflags)
> +struct property *of_property_alloc(const char *name, const void *value,
> +int value_len, int len, gfp_t allocflags)
> {
> - struct property *new;
> + int alloc_len = len;
> + struct property *prop;
> +
> + if (len < value_len)
> + return NULL;
>
> - new = kzalloc(sizeof(*new), allocflags);
> - if (!new)
> + prop = kzalloc(sizeof(*prop), allocflags);
> + if (!prop)
> return NULL;
>
> + prop->name = kstrdup(name, allocflags);
> + if (!prop->name)
> + goto out_err;
> +
> /*
> - * NOTE: There is no check for zero length value.
> - * In case of a boolean property, this will allocate a value
> - * of zero bytes. We do this to work around the use
> - * of of_get_property() calls on boolean values.
> + * Even if the property has no value, it must be set to a
> + * non-null value since of_get_property() is used to check
> + * some values that might or not have a values (ranges for
> + * instance). Moreover, when the node is released, prop->value
> + * is kfreed so the memory must come from kmalloc.
Allowing for NULL value didn't turn out well...
We know that we can do the kfree because OF_DYNAMIC is set IIRC...
If we do 1 allocation for prop and value, then we can test
for "prop->value == prop + 1" to determine if we need to free or not.
>*/
> - new->name = kstrdup(prop->name, allocflags);
> - new->value = kmemdup(prop->value, prop->length, allocflags);
> - new->length = prop->length;
> - if (!new->name || !new->value)
> - goto err_free;
> + if (!alloc_len)
> + alloc_len = 1;
>
> - /* mark the property as dynamic */
> - of_property_set_flag(new, OF_DYNAMIC);
> + prop->value = kzalloc(alloc_len, allocflags);
> + if (!prop->value)
> + goto out_err;
>
> - return new;
> + if (value)
> + memcpy(prop->value, value, value_len);
> +
> + prop->length = len;
> + of_property_set_flag(prop, OF_DYNAMIC);
> +
> + return prop;
> +
> +out_err:
> + of_property_free(prop);
>
> - err_free:
> - kfree(new->name);
> - kfree(new->value);
> - kfree(new);
> return NULL;
> }
> +EXPORT_SYMBOL(of_property_alloc);
> +
> +/**
> + * __of_prop_dup - Copy a property dynamically.
> + * @prop:Property to copy
> + * @allocflags: Allocation flags
Re: [PATCH 1/3] of: dynamic: add of_property_alloc() and of_property_free()
On Thu, May 05, 2022 at 07:30:47AM +, Christophe Leroy wrote:
>
>
> Le 04/05/2022 à 17:40, Clément Léger a écrit :
> > Add function which allows to dynamically allocate and free properties.
> > Use this function internally for all code that used the same logic
> > (mainly __of_prop_dup()).
> >
> > Signed-off-by: Clément Léger
> > ---
> > drivers/of/dynamic.c | 101 ++-
> > include/linux/of.h | 16 +++
> > 2 files changed, 88 insertions(+), 29 deletions(-)
> >
> > diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c
> > index cd3821a6444f..e8700e509d2e 100644
> > --- a/drivers/of/dynamic.c
> > +++ b/drivers/of/dynamic.c
> > @@ -313,9 +313,7 @@ static void property_list_free(struct property
> > *prop_list)
> >
> > for (prop = prop_list; prop != NULL; prop = next) {
> > next = prop->next;
> > - kfree(prop->name);
> > - kfree(prop->value);
> > - kfree(prop);
> > + of_property_free(prop);
> > }
> > }
> >
> > @@ -367,48 +365,95 @@ void of_node_release(struct kobject *kobj)
> > }
> >
> > /**
> > - * __of_prop_dup - Copy a property dynamically.
> > - * @prop: Property to copy
> > + * of_property_free - Free a property allocated dynamically.
> > + * @prop: Property to be freed
> > + */
> > +void of_property_free(const struct property *prop)
> > +{
> > + kfree(prop->value);
> > + kfree(prop->name);
> > + kfree(prop);
> > +}
> > +EXPORT_SYMBOL(of_property_free);
> > +
> > +/**
> > + * of_property_alloc - Allocate a property dynamically.
> > + * @name: Name of the new property
> > + * @value: Value that will be copied into the new property value
> > + * @value_len: length of @value to be copied into the new property
> > value
> > + * @len: Length of new property value, must be greater than @value_len
> >* @allocflags: Allocation flags (typically pass GFP_KERNEL)
> >*
> > - * Copy a property by dynamically allocating the memory of both the
> > + * Create a property by dynamically allocating the memory of both the
> >* property structure and the property name & contents. The property's
> >* flags have the OF_DYNAMIC bit set so that we can differentiate between
> >* dynamically allocated properties and not.
> >*
> >* Return: The newly allocated property or NULL on out of memory error.
> >*/
> > -struct property *__of_prop_dup(const struct property *prop, gfp_t
> > allocflags)
> > +struct property *of_property_alloc(const char *name, const void *value,
> > + int value_len, int len, gfp_t allocflags)
> > {
> > - struct property *new;
> > + int alloc_len = len;
> > + struct property *prop;
> > +
> > + if (len < value_len)
> > + return NULL;
> >
> > - new = kzalloc(sizeof(*new), allocflags);
> > - if (!new)
> > + prop = kzalloc(sizeof(*prop), allocflags);
> > + if (!prop)
> > return NULL;
> >
> > + prop->name = kstrdup(name, allocflags);
> > + if (!prop->name)
> > + goto out_err;
> > +
> > /*
> > -* NOTE: There is no check for zero length value.
> > -* In case of a boolean property, this will allocate a value
> > -* of zero bytes. We do this to work around the use
> > -* of of_get_property() calls on boolean values.
> > +* Even if the property has no value, it must be set to a
> > +* non-null value since of_get_property() is used to check
> > +* some values that might or not have a values (ranges for
> > +* instance). Moreover, when the node is released, prop->value
> > +* is kfreed so the memory must come from kmalloc.
> > */
> > - new->name = kstrdup(prop->name, allocflags);
> > - new->value = kmemdup(prop->value, prop->length, allocflags);
> > - new->length = prop->length;
> > - if (!new->name || !new->value)
> > - goto err_free;
> > + if (!alloc_len)
> > + alloc_len = 1;
> >
> > - /* mark the property as dynamic */
> > - of_property_set_flag(new, OF_DYNAMIC);
> > + prop->value = kzalloc(alloc_len, allocflags);
> > + if (!prop->value)
> > + goto out_err;
> >
> > - return new;
> > + if (value)
> > + memcpy(prop->value, value, value_len);
>
> Could you use kmemdup() instead of kzalloc+memcpy ?
I'd prefer there be 1 alloc for struct property and value instead of 2.
And maybe 'name' gets rolled into it too, but that gets a bit more
complicated to manage I think.
With memcpy, note this series[1].
Rob
[1] https://lore.kernel.org/all/20220504014440.3697851-30-keesc...@chromium.org/
Re: [PATCH 1/3] of: dynamic: add of_property_alloc() and of_property_free()
Le 04/05/2022 à 17:40, Clément Léger a écrit :
> Add function which allows to dynamically allocate and free properties.
> Use this function internally for all code that used the same logic
> (mainly __of_prop_dup()).
>
> Signed-off-by: Clément Léger
> ---
> drivers/of/dynamic.c | 101 ++-
> include/linux/of.h | 16 +++
> 2 files changed, 88 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c
> index cd3821a6444f..e8700e509d2e 100644
> --- a/drivers/of/dynamic.c
> +++ b/drivers/of/dynamic.c
> @@ -313,9 +313,7 @@ static void property_list_free(struct property *prop_list)
>
> for (prop = prop_list; prop != NULL; prop = next) {
> next = prop->next;
> - kfree(prop->name);
> - kfree(prop->value);
> - kfree(prop);
> + of_property_free(prop);
> }
> }
>
> @@ -367,48 +365,95 @@ void of_node_release(struct kobject *kobj)
> }
>
> /**
> - * __of_prop_dup - Copy a property dynamically.
> - * @prop:Property to copy
> + * of_property_free - Free a property allocated dynamically.
> + * @prop:Property to be freed
> + */
> +void of_property_free(const struct property *prop)
> +{
> + kfree(prop->value);
> + kfree(prop->name);
> + kfree(prop);
> +}
> +EXPORT_SYMBOL(of_property_free);
> +
> +/**
> + * of_property_alloc - Allocate a property dynamically.
> + * @name:Name of the new property
> + * @value: Value that will be copied into the new property value
> + * @value_len: length of @value to be copied into the new property
> value
> + * @len: Length of new property value, must be greater than @value_len
>* @allocflags: Allocation flags (typically pass GFP_KERNEL)
>*
> - * Copy a property by dynamically allocating the memory of both the
> + * Create a property by dynamically allocating the memory of both the
>* property structure and the property name & contents. The property's
>* flags have the OF_DYNAMIC bit set so that we can differentiate between
>* dynamically allocated properties and not.
>*
>* Return: The newly allocated property or NULL on out of memory error.
>*/
> -struct property *__of_prop_dup(const struct property *prop, gfp_t allocflags)
> +struct property *of_property_alloc(const char *name, const void *value,
> +int value_len, int len, gfp_t allocflags)
> {
> - struct property *new;
> + int alloc_len = len;
> + struct property *prop;
> +
> + if (len < value_len)
> + return NULL;
>
> - new = kzalloc(sizeof(*new), allocflags);
> - if (!new)
> + prop = kzalloc(sizeof(*prop), allocflags);
> + if (!prop)
> return NULL;
>
> + prop->name = kstrdup(name, allocflags);
> + if (!prop->name)
> + goto out_err;
> +
> /*
> - * NOTE: There is no check for zero length value.
> - * In case of a boolean property, this will allocate a value
> - * of zero bytes. We do this to work around the use
> - * of of_get_property() calls on boolean values.
> + * Even if the property has no value, it must be set to a
> + * non-null value since of_get_property() is used to check
> + * some values that might or not have a values (ranges for
> + * instance). Moreover, when the node is released, prop->value
> + * is kfreed so the memory must come from kmalloc.
>*/
> - new->name = kstrdup(prop->name, allocflags);
> - new->value = kmemdup(prop->value, prop->length, allocflags);
> - new->length = prop->length;
> - if (!new->name || !new->value)
> - goto err_free;
> + if (!alloc_len)
> + alloc_len = 1;
>
> - /* mark the property as dynamic */
> - of_property_set_flag(new, OF_DYNAMIC);
> + prop->value = kzalloc(alloc_len, allocflags);
> + if (!prop->value)
> + goto out_err;
>
> - return new;
> + if (value)
> + memcpy(prop->value, value, value_len);
Could you use kmemdup() instead of kzalloc+memcpy ?
> +
> + prop->length = len;
> + of_property_set_flag(prop, OF_DYNAMIC);
> +
> + return prop;
> +
> +out_err:
> + of_property_free(prop);
>
> - err_free:
> - kfree(new->name);
> - kfree(new->value);
> - kfree(new);
> return NULL;
> }
> +EXPORT_SYMBOL(of_property_alloc);
> +
> +/**
> + * __of_prop_dup - Copy a property dynamically.
> + * @prop:Property to copy
> + * @allocflags: Allocation flags (typically pass GFP_KERNEL)
> + *
> + * Copy a property by dynamically allocating the memory of both the
> + * property structure and the property name & contents. The property's
> + * flags have the OF_DYNAMIC bit set so that we can differentiate between
> + * dynamically allocated properties and not.
> + *
> + * Return: The newly allocated property or NULL on out of memory error.
> + */
>
