[PATCH] Introduce fdt_setprop_placeholder() method

[PATCH] Introduce fdt_setprop_placeholder() method

[Pantelis Antoniou]

In some cases you need to add a property but the contents of it
are not known at creation time, merely the extend of it.

This method allows you to create a property of a given size
which will be set to zero while a pointer to the property data
will be provided.

Signed-off-by: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
---
 libfdt/fdt_rw.c | 21 +++++++++++++++++++++
 libfdt/libfdt.h | 32 ++++++++++++++++++++++++++++++++
 2 files changed, 53 insertions(+)

diff --git a/libfdt/fdt_rw.c b/libfdt/fdt_rw.c
index 8b487f6..2b4432f 100644
--- a/libfdt/fdt_rw.c
+++ b/libfdt/fdt_rw.c
@@ -288,6 +288,27 @@ int fdt_setprop(void *fdt, int nodeoffset, const char *name,
 	return 0;
 }
 
+int fdt_setprop_placeholder(void *fdt, int nodeoffset, const char *name,
+			    int len, void **prop_data)
+{
+	struct fdt_property *prop;
+	int err;
+
+	FDT_RW_CHECK_HEADER(fdt);
+
+	err = _fdt_resize_property(fdt, nodeoffset, name, len, &prop);
+	if (err == -FDT_ERR_NOTFOUND)
+		err = _fdt_add_property(fdt, nodeoffset, name, len, &prop);
+	if (err)
+		return err;
+
+	if (len)
+		memset(prop->data, 0, len);
+	if (prop_data)
+		*prop_data = prop->data;
+	return 0;
+}
+
 int fdt_appendprop(void *fdt, int nodeoffset, const char *name,
 		   const void *val, int len)
 {
diff --git a/libfdt/libfdt.h b/libfdt/libfdt.h
index a248b1b..7ea9a0c 100644
--- a/libfdt/libfdt.h
+++ b/libfdt/libfdt.h
@@ -1449,6 +1449,38 @@ int fdt_setprop(void *fdt, int nodeoffset, const char *name,
 		const void *val, int len);
 
 /**
+ * fdt_setprop _placeholder - allocate space for a property
+ * @fdt: pointer to the device tree blob
+ * @nodeoffset: offset of the node whose property to change
+ * @name: name of the property to change
+ * @len: length of the property value
+ * @prop_data: return pointer to property data
+ *
+ * fdt_setprop_placeholer() allocates the named property in the given node.
+ * If the property exists it is resized. In either case it filled with
+ * zeroes and if a prop_data argument is provided a pointer to the
+ * property data is returned.
+ *
+ * This function may insert or delete data from the blob, and will
+ * therefore change the offsets of some existing nodes.
+ *
+ * returns:
+ *	0, on success
+ *	-FDT_ERR_NOSPACE, there is insufficient free space in the blob to
+ *		contain the new property value
+ *	-FDT_ERR_BADOFFSET, nodeoffset did not point to FDT_BEGIN_NODE tag
+ *	-FDT_ERR_BADLAYOUT,
+ *	-FDT_ERR_BADMAGIC,
+ *	-FDT_ERR_BADVERSION,
+ *	-FDT_ERR_BADSTATE,
+ *	-FDT_ERR_BADSTRUCTURE,
+ *	-FDT_ERR_BADLAYOUT,
+ *	-FDT_ERR_TRUNCATED, standard meanings
+ */
+int fdt_setprop_placeholder(void *fdt, int nodeoffset, const char *name,
+			    int len, void **prop_data);
+
+/**
  * fdt_setprop_u32 - set a property to a 32-bit integer
  * @fdt: pointer to the device tree blob
  * @nodeoffset: offset of the node whose property to change
-- 
2.1.4

Re: [PATCH] Introduce fdt_setprop_placeholder() method

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 3912 bytes --]

On Tue, Jul 25, 2017 at 08:24:21PM +0300, Pantelis Antoniou wrote:
> In some cases you need to add a property but the contents of it
> are not known at creation time, merely the extend of it.
> 
> This method allows you to create a property of a given size
> which will be set to zero while a pointer to the property data
> will be provided.
> 
> Signed-off-by: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>

Almost there, couple of details to fix.

> ---
>  libfdt/fdt_rw.c | 21 +++++++++++++++++++++
>  libfdt/libfdt.h | 32 ++++++++++++++++++++++++++++++++
>  2 files changed, 53 insertions(+)
> 
> diff --git a/libfdt/fdt_rw.c b/libfdt/fdt_rw.c
> index 8b487f6..2b4432f 100644
> --- a/libfdt/fdt_rw.c
> +++ b/libfdt/fdt_rw.c
> @@ -288,6 +288,27 @@ int fdt_setprop(void *fdt, int nodeoffset, const char *name,
>  	return 0;
>  }
>  
> +int fdt_setprop_placeholder(void *fdt, int nodeoffset, const char *name,
> +			    int len, void **prop_data)
> +{
> +	struct fdt_property *prop;
> +	int err;
> +
> +	FDT_RW_CHECK_HEADER(fdt);
> +
> +	err = _fdt_resize_property(fdt, nodeoffset, name, len, &prop);
> +	if (err == -FDT_ERR_NOTFOUND)
> +		err = _fdt_add_property(fdt, nodeoffset, name, len, &prop);
> +	if (err)
> +		return err;
> +
> +	if (len)
> +		memset(prop->data, 0, len);

I don't think we want the memset - the caller is expected to overwrite
the content anyway.  That also patches fdt_property_placeholder()
which doesn't clear the allocated space.

> +	if (prop_data)
> +		*prop_data = prop->data;

Likewise, I'd make the prop_data parameter mandatory, so no need for a
NULL check.  Again, this patches fdt_property_placeholder().

You can probably also simplify the existing fdt_setprop() function a
little by using fdt_setprop_placeholder() within it.

> +	return 0;
> +}
> +
>  int fdt_appendprop(void *fdt, int nodeoffset, const char *name,
>  		   const void *val, int len)
>  {
> diff --git a/libfdt/libfdt.h b/libfdt/libfdt.h
> index a248b1b..7ea9a0c 100644
> --- a/libfdt/libfdt.h
> +++ b/libfdt/libfdt.h
> @@ -1449,6 +1449,38 @@ int fdt_setprop(void *fdt, int nodeoffset, const char *name,
>  		const void *val, int len);
>  
>  /**
> + * fdt_setprop _placeholder - allocate space for a property
> + * @fdt: pointer to the device tree blob
> + * @nodeoffset: offset of the node whose property to change
> + * @name: name of the property to change
> + * @len: length of the property value
> + * @prop_data: return pointer to property data
> + *
> + * fdt_setprop_placeholer() allocates the named property in the given node.
> + * If the property exists it is resized. In either case it filled with
> + * zeroes and if a prop_data argument is provided a pointer to the
> + * property data is returned.
> + *
> + * This function may insert or delete data from the blob, and will
> + * therefore change the offsets of some existing nodes.
> + *
> + * returns:
> + *	0, on success
> + *	-FDT_ERR_NOSPACE, there is insufficient free space in the blob to
> + *		contain the new property value
> + *	-FDT_ERR_BADOFFSET, nodeoffset did not point to FDT_BEGIN_NODE tag
> + *	-FDT_ERR_BADLAYOUT,
> + *	-FDT_ERR_BADMAGIC,
> + *	-FDT_ERR_BADVERSION,
> + *	-FDT_ERR_BADSTATE,
> + *	-FDT_ERR_BADSTRUCTURE,
> + *	-FDT_ERR_BADLAYOUT,
> + *	-FDT_ERR_TRUNCATED, standard meanings
> + */
> +int fdt_setprop_placeholder(void *fdt, int nodeoffset, const char *name,
> +			    int len, void **prop_data);
> +
> +/**
>   * fdt_setprop_u32 - set a property to a 32-bit integer
>   * @fdt: pointer to the device tree blob
>   * @nodeoffset: offset of the node whose property to change

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH] Introduce fdt_setprop_placeholder() method

[Pantelis Antoniou]

In some cases you need to add a property but the contents of it
are not known at creation time, merely the extend of it.

This method allows you to create a property of a given size
which will be set to zero while a pointer to the property data
will be provided.

Signed-off-by: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
---
 libfdt/fdt_rw.c | 20 +++++++++++++++++---
 libfdt/libfdt.h | 32 ++++++++++++++++++++++++++++++++
 2 files changed, 49 insertions(+), 3 deletions(-)

diff --git a/libfdt/fdt_rw.c b/libfdt/fdt_rw.c
index 8b487f6..5c3a2bb 100644
--- a/libfdt/fdt_rw.c
+++ b/libfdt/fdt_rw.c
@@ -269,8 +269,8 @@ int fdt_set_name(void *fdt, int nodeoffset, const char *name)
 	return 0;
 }
 
-int fdt_setprop(void *fdt, int nodeoffset, const char *name,
-		const void *val, int len)
+int fdt_setprop_placeholder(void *fdt, int nodeoffset, const char *name,
+			    int len, void **prop_data)
 {
 	struct fdt_property *prop;
 	int err;
@@ -283,8 +283,22 @@ int fdt_setprop(void *fdt, int nodeoffset, const char *name,
 	if (err)
 		return err;
 
+	*prop_data = prop->data;
+	return 0;
+}
+
+int fdt_setprop(void *fdt, int nodeoffset, const char *name,
+		const void *val, int len)
+{
+	void *prop_data;
+	int err;
+
+	err = fdt_setprop_placeholder(fdt, nodeoffset, name, len, &prop_data);
+	if (err)
+		return err;
+
 	if (len)
-		memcpy(prop->data, val, len);
+		memcpy(prop_data, val, len);
 	return 0;
 }
 
diff --git a/libfdt/libfdt.h b/libfdt/libfdt.h
index a248b1b..7ea9a0c 100644
--- a/libfdt/libfdt.h
+++ b/libfdt/libfdt.h
@@ -1449,6 +1449,38 @@ int fdt_setprop(void *fdt, int nodeoffset, const char *name,
 		const void *val, int len);
 
 /**
+ * fdt_setprop _placeholder - allocate space for a property
+ * @fdt: pointer to the device tree blob
+ * @nodeoffset: offset of the node whose property to change
+ * @name: name of the property to change
+ * @len: length of the property value
+ * @prop_data: return pointer to property data
+ *
+ * fdt_setprop_placeholer() allocates the named property in the given node.
+ * If the property exists it is resized. In either case it filled with
+ * zeroes and if a prop_data argument is provided a pointer to the
+ * property data is returned.
+ *
+ * This function may insert or delete data from the blob, and will
+ * therefore change the offsets of some existing nodes.
+ *
+ * returns:
+ *	0, on success
+ *	-FDT_ERR_NOSPACE, there is insufficient free space in the blob to
+ *		contain the new property value
+ *	-FDT_ERR_BADOFFSET, nodeoffset did not point to FDT_BEGIN_NODE tag
+ *	-FDT_ERR_BADLAYOUT,
+ *	-FDT_ERR_BADMAGIC,
+ *	-FDT_ERR_BADVERSION,
+ *	-FDT_ERR_BADSTATE,
+ *	-FDT_ERR_BADSTRUCTURE,
+ *	-FDT_ERR_BADLAYOUT,
+ *	-FDT_ERR_TRUNCATED, standard meanings
+ */
+int fdt_setprop_placeholder(void *fdt, int nodeoffset, const char *name,
+			    int len, void **prop_data);
+
+/**
  * fdt_setprop_u32 - set a property to a 32-bit integer
  * @fdt: pointer to the device tree blob
  * @nodeoffset: offset of the node whose property to change
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Introduce fdt_setprop_placeholder() method

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 2203 bytes --]

On Wed, Jul 26, 2017 at 05:46:58PM +0300, Pantelis Antoniou wrote:
> In some cases you need to add a property but the contents of it
> are not known at creation time, merely the extend of it.
> 
> This method allows you to create a property of a given size
> which will be set to zero while a pointer to the property data
> will be provided.
> 
> Signed-off-by: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
> ---
[snip]
>  /**
> + * fdt_setprop _placeholder - allocate space for a property
> + * @fdt: pointer to the device tree blob
> + * @nodeoffset: offset of the node whose property to change
> + * @name: name of the property to change
> + * @len: length of the property value
> + * @prop_data: return pointer to property data
> + *
> + * fdt_setprop_placeholer() allocates the named property in the given node.
> + * If the property exists it is resized. In either case it filled with
> + * zeroes and if a prop_data argument is provided a pointer to the
> + * property data is returned.

Doc comment hasn't been updated to reflect the latest chnages (not zeroed).

> + * This function may insert or delete data from the blob, and will
> + * therefore change the offsets of some existing nodes.
> + *
> + * returns:
> + *	0, on success
> + *	-FDT_ERR_NOSPACE, there is insufficient free space in the blob to
> + *		contain the new property value
> + *	-FDT_ERR_BADOFFSET, nodeoffset did not point to FDT_BEGIN_NODE tag
> + *	-FDT_ERR_BADLAYOUT,
> + *	-FDT_ERR_BADMAGIC,
> + *	-FDT_ERR_BADVERSION,
> + *	-FDT_ERR_BADSTATE,
> + *	-FDT_ERR_BADSTRUCTURE,
> + *	-FDT_ERR_BADLAYOUT,
> + *	-FDT_ERR_TRUNCATED, standard meanings
> + */
> +int fdt_setprop_placeholder(void *fdt, int nodeoffset, const char *name,
> +			    int len, void **prop_data);
> +
> +/**
>   * fdt_setprop_u32 - set a property to a 32-bit integer
>   * @fdt: pointer to the device tree blob
>   * @nodeoffset: offset of the node whose property to change

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH] Introduce fdt_setprop_placeholder() method

[Pantelis Antoniou]

In some cases you need to add a property but the contents of it
are not known at creation time, merely the extend of it.

This method allows you to create a property of a given size
which will be set to zero while a pointer to the property data
will be provided.

Signed-off-by: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
---
 libfdt/fdt_rw.c | 20 +++++++++++++++++---
 libfdt/libfdt.h | 31 +++++++++++++++++++++++++++++++
 2 files changed, 48 insertions(+), 3 deletions(-)

diff --git a/libfdt/fdt_rw.c b/libfdt/fdt_rw.c
index 8b487f6..5c3a2bb 100644
--- a/libfdt/fdt_rw.c
+++ b/libfdt/fdt_rw.c
@@ -269,8 +269,8 @@ int fdt_set_name(void *fdt, int nodeoffset, const char *name)
 	return 0;
 }
 
-int fdt_setprop(void *fdt, int nodeoffset, const char *name,
-		const void *val, int len)
+int fdt_setprop_placeholder(void *fdt, int nodeoffset, const char *name,
+			    int len, void **prop_data)
 {
 	struct fdt_property *prop;
 	int err;
@@ -283,8 +283,22 @@ int fdt_setprop(void *fdt, int nodeoffset, const char *name,
 	if (err)
 		return err;
 
+	*prop_data = prop->data;
+	return 0;
+}
+
+int fdt_setprop(void *fdt, int nodeoffset, const char *name,
+		const void *val, int len)
+{
+	void *prop_data;
+	int err;
+
+	err = fdt_setprop_placeholder(fdt, nodeoffset, name, len, &prop_data);
+	if (err)
+		return err;
+
 	if (len)
-		memcpy(prop->data, val, len);
+		memcpy(prop_data, val, len);
 	return 0;
 }
 
diff --git a/libfdt/libfdt.h b/libfdt/libfdt.h
index a248b1b..313c72a 100644
--- a/libfdt/libfdt.h
+++ b/libfdt/libfdt.h
@@ -1449,6 +1449,37 @@ int fdt_setprop(void *fdt, int nodeoffset, const char *name,
 		const void *val, int len);
 
 /**
+ * fdt_setprop _placeholder - allocate space for a property
+ * @fdt: pointer to the device tree blob
+ * @nodeoffset: offset of the node whose property to change
+ * @name: name of the property to change
+ * @len: length of the property value
+ * @prop_data: return pointer to property data
+ *
+ * fdt_setprop_placeholer() allocates the named property in the given node.
+ * If the property exists it is resized. In either case a pointer to the
+ * property data is returned.
+ *
+ * This function may insert or delete data from the blob, and will
+ * therefore change the offsets of some existing nodes.
+ *
+ * returns:
+ *	0, on success
+ *	-FDT_ERR_NOSPACE, there is insufficient free space in the blob to
+ *		contain the new property value
+ *	-FDT_ERR_BADOFFSET, nodeoffset did not point to FDT_BEGIN_NODE tag
+ *	-FDT_ERR_BADLAYOUT,
+ *	-FDT_ERR_BADMAGIC,
+ *	-FDT_ERR_BADVERSION,
+ *	-FDT_ERR_BADSTATE,
+ *	-FDT_ERR_BADSTRUCTURE,
+ *	-FDT_ERR_BADLAYOUT,
+ *	-FDT_ERR_TRUNCATED, standard meanings
+ */
+int fdt_setprop_placeholder(void *fdt, int nodeoffset, const char *name,
+			    int len, void **prop_data);
+
+/**
  * fdt_setprop_u32 - set a property to a 32-bit integer
  * @fdt: pointer to the device tree blob
  * @nodeoffset: offset of the node whose property to change
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Introduce fdt_setprop_placeholder() method

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 3768 bytes --]

On Thu, Jul 27, 2017 at 07:33:11PM +0300, Pantelis Antoniou wrote:
> In some cases you need to add a property but the contents of it
> are not known at creation time, merely the extend of it.
> 
> This method allows you to create a property of a given size
> which will be set to zero while a pointer to the property data
> will be provided.

Commit message still incorrectly refers to zeroing the property.

I've corrected it and applied.  But review comments are not sed
scripts - you're supposed to see what changes logically follow from
those suggested and make those as well.

> Signed-off-by: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
> ---
>  libfdt/fdt_rw.c | 20 +++++++++++++++++---
>  libfdt/libfdt.h | 31 +++++++++++++++++++++++++++++++
>  2 files changed, 48 insertions(+), 3 deletions(-)
> 
> diff --git a/libfdt/fdt_rw.c b/libfdt/fdt_rw.c
> index 8b487f6..5c3a2bb 100644
> --- a/libfdt/fdt_rw.c
> +++ b/libfdt/fdt_rw.c
> @@ -269,8 +269,8 @@ int fdt_set_name(void *fdt, int nodeoffset, const char *name)
>  	return 0;
>  }
>  
> -int fdt_setprop(void *fdt, int nodeoffset, const char *name,
> -		const void *val, int len)
> +int fdt_setprop_placeholder(void *fdt, int nodeoffset, const char *name,
> +			    int len, void **prop_data)
>  {
>  	struct fdt_property *prop;
>  	int err;
> @@ -283,8 +283,22 @@ int fdt_setprop(void *fdt, int nodeoffset, const char *name,
>  	if (err)
>  		return err;
>  
> +	*prop_data = prop->data;
> +	return 0;
> +}
> +
> +int fdt_setprop(void *fdt, int nodeoffset, const char *name,
> +		const void *val, int len)
> +{
> +	void *prop_data;
> +	int err;
> +
> +	err = fdt_setprop_placeholder(fdt, nodeoffset, name, len, &prop_data);
> +	if (err)
> +		return err;
> +
>  	if (len)
> -		memcpy(prop->data, val, len);
> +		memcpy(prop_data, val, len);
>  	return 0;
>  }
>  
> diff --git a/libfdt/libfdt.h b/libfdt/libfdt.h
> index a248b1b..313c72a 100644
> --- a/libfdt/libfdt.h
> +++ b/libfdt/libfdt.h
> @@ -1449,6 +1449,37 @@ int fdt_setprop(void *fdt, int nodeoffset, const char *name,
>  		const void *val, int len);
>  
>  /**
> + * fdt_setprop _placeholder - allocate space for a property
> + * @fdt: pointer to the device tree blob
> + * @nodeoffset: offset of the node whose property to change
> + * @name: name of the property to change
> + * @len: length of the property value
> + * @prop_data: return pointer to property data
> + *
> + * fdt_setprop_placeholer() allocates the named property in the given node.
> + * If the property exists it is resized. In either case a pointer to the
> + * property data is returned.
> + *
> + * This function may insert or delete data from the blob, and will
> + * therefore change the offsets of some existing nodes.
> + *
> + * returns:
> + *	0, on success
> + *	-FDT_ERR_NOSPACE, there is insufficient free space in the blob to
> + *		contain the new property value
> + *	-FDT_ERR_BADOFFSET, nodeoffset did not point to FDT_BEGIN_NODE tag
> + *	-FDT_ERR_BADLAYOUT,
> + *	-FDT_ERR_BADMAGIC,
> + *	-FDT_ERR_BADVERSION,
> + *	-FDT_ERR_BADSTATE,
> + *	-FDT_ERR_BADSTRUCTURE,
> + *	-FDT_ERR_BADLAYOUT,
> + *	-FDT_ERR_TRUNCATED, standard meanings
> + */
> +int fdt_setprop_placeholder(void *fdt, int nodeoffset, const char *name,
> +			    int len, void **prop_data);
> +
> +/**
>   * fdt_setprop_u32 - set a property to a 32-bit integer
>   * @fdt: pointer to the device tree blob
>   * @nodeoffset: offset of the node whose property to change

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]