[PATCH 0/4] libfdt: Add namelen variants for setprop

[PATCH 0/4] libfdt: Add namelen variants for setprop

[Ayush Singh]

Helper functions to setproperty with length of property name similar to
getprop_namelen variants.

This patch series was originally part of overlay-path patch series [0].

[0]: https://lore.kernel.org/devicetree-compiler/6b2dba90-3c52-4933-88f3-b47f96dc7710@beagleboard.org/T/#t

Signed-off-by: Ayush Singh <ayush@beagleboard.org>

---
Ayush Singh (4):
      libfdt: add fdt_get_property_namelen_w()
      libfdt_internal: fdt_find_string_len_()
      libfdt: Add fdt_setprop_namelen()
      libfdt: Add fdt_setprop_namelen_string()

 libfdt/fdt.c             |   8 ++--
 libfdt/fdt_rw.c          |  41 +++++++++-------
 libfdt/libfdt.h          | 120 +++++++++++++++++++++++++++++++++++++++++++++--
 libfdt/libfdt_internal.h |  11 ++++-
 libfdt/version.lds       |   2 +
 5 files changed, 156 insertions(+), 26 deletions(-)
---
base-commit: 6f183c7d9246bde8f05d2edbc31fdd497c4a8702
change-id: 20241203-setprop-namelen-a94f7f057acf

Best regards,
-- 
Ayush Singh <ayush@beagleboard.org>

[PATCH 1/4] libfdt: add fdt_get_property_namelen_w()

[Ayush Singh]

Similar to the non-namelen variant, it is implemented in terms of
fdt_get_property_namelen()

Signed-off-by: Ayush Singh <ayush@beagleboard.org>
---
 libfdt/libfdt.h | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/libfdt/libfdt.h b/libfdt/libfdt.h
index 2e182ea3314d44f30e5362ad1ca62431c0bf663b..7d0c252a34970f4ca841935f8088410a3970127a 100644
--- a/libfdt/libfdt.h
+++ b/libfdt/libfdt.h
@@ -712,6 +712,13 @@ const struct fdt_property *fdt_get_property_namelen(const void *fdt,
 						    int nodeoffset,
 						    const char *name,
 						    int namelen, int *lenp);
+static inline struct fdt_property *
+fdt_get_property_namelen_w(void *fdt, int nodeoffset, const char *name,
+			   int namelen, int *lenp)
+{
+	return (struct fdt_property *)(uintptr_t)fdt_get_property_namelen(
+		fdt, nodeoffset, name, namelen, lenp);
+}
 #endif
 
 /**

-- 
2.47.1

[PATCH 2/4] libfdt_internal: fdt_find_string_len_()

[Ayush Singh]

Allow specifying string length to `fdt_find_string_`.
fdt_find_string_() now internally uses this function.

Signed-off-by: Ayush Singh <ayush@beagleboard.org>
---
 libfdt/fdt.c             |  8 ++++----
 libfdt/libfdt_internal.h | 11 ++++++++++-
 2 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/libfdt/fdt.c b/libfdt/fdt.c
index 20c6415b9ced113d9fcce25b69007a6b0f9e68e7..95f644c31f9431eb597b6f683741ff9be375cebf 100644
--- a/libfdt/fdt.c
+++ b/libfdt/fdt.c
@@ -312,14 +312,14 @@ int fdt_next_subnode(const void *fdt, int offset)
 	return offset;
 }
 
-const char *fdt_find_string_(const char *strtab, int tabsize, const char *s)
+const char *fdt_find_string_len_(const char *strtab, int tabsize, const char *s,
+				 int slen)
 {
-	int len = strlen(s) + 1;
-	const char *last = strtab + tabsize - len;
+	const char *last = strtab + tabsize - (slen + 1);
 	const char *p;
 
 	for (p = strtab; p <= last; p++)
-		if (memcmp(p, s, len) == 0)
+		if (memcmp(p, s, slen) == 0 && p[slen] == '\0')
 			return p;
 	return NULL;
 }
diff --git a/libfdt/libfdt_internal.h b/libfdt/libfdt_internal.h
index 16bda1906a7b335519b9f748d1be6110de551e79..1fd35f364e9af15df19ae5b8b7231584804e96a4 100644
--- a/libfdt/libfdt_internal.h
+++ b/libfdt/libfdt_internal.h
@@ -6,6 +6,7 @@
  * Copyright (C) 2006 David Gibson, IBM Corporation.
  */
 #include <fdt.h>
+#include <string.h>
 
 #define FDT_ALIGN(x, a)		(((x) + (a) - 1) & ~((a) - 1))
 #define FDT_TAGALIGN(x)		(FDT_ALIGN((x), FDT_TAGSIZE))
@@ -20,7 +21,15 @@ int32_t fdt_ro_probe_(const void *fdt);
 
 int fdt_check_node_offset_(const void *fdt, int offset);
 int fdt_check_prop_offset_(const void *fdt, int offset);
-const char *fdt_find_string_(const char *strtab, int tabsize, const char *s);
+
+const char *fdt_find_string_len_(const char *strtab, int tabsize, const char *s,
+				 int s_len);
+static inline const char *fdt_find_string_(const char *strtab, int tabsize,
+					   const char *s)
+{
+	return fdt_find_string_len_(strtab, tabsize, s, strlen(s));
+}
+
 int fdt_node_end_offset_(void *fdt, int nodeoffset);
 
 static inline const void *fdt_offset_ptr_(const void *fdt, int offset)

-- 
2.47.1

[PATCH 3/4] libfdt: Add fdt_setprop_namelen()

[Ayush Singh]

Allow specifying name length in setprop similar to
`fdt_get_property_namelen` functions.

Signed-off-by: Ayush Singh <ayush@beagleboard.org>
---
 libfdt/fdt_rw.c    | 41 +++++++++++++++------------
 libfdt/libfdt.h    | 81 +++++++++++++++++++++++++++++++++++++++++++++++++++---
 libfdt/version.lds |  2 ++
 3 files changed, 103 insertions(+), 21 deletions(-)

diff --git a/libfdt/fdt_rw.c b/libfdt/fdt_rw.c
index 3621d3651d3f4bd82b7af66c60d023e3139add03..33c60063cd721a147a3c6c0e70450c98d8dbd772 100644
--- a/libfdt/fdt_rw.c
+++ b/libfdt/fdt_rw.c
@@ -124,31 +124,33 @@ static int fdt_splice_string_(void *fdt, int newlen)
  *	allocated. Ignored if can_assume(NO_ROLLBACK)
  * @return offset of string in the string table (whether found or added)
  */
-static int fdt_find_add_string_(void *fdt, const char *s, int *allocated)
+static int fdt_find_add_string_(void *fdt, const char *s, int slen,
+				int *allocated)
 {
 	char *strtab = (char *)fdt + fdt_off_dt_strings(fdt);
 	const char *p;
 	char *new;
-	int len = strlen(s) + 1;
 	int err;
 
 	if (!can_assume(NO_ROLLBACK))
 		*allocated = 0;
 
-	p = fdt_find_string_(strtab, fdt_size_dt_strings(fdt), s);
+	p = fdt_find_string_len_(strtab, fdt_size_dt_strings(fdt), s, slen);
 	if (p)
 		/* found it */
 		return (p - strtab);
 
 	new = strtab + fdt_size_dt_strings(fdt);
-	err = fdt_splice_string_(fdt, len);
+	err = fdt_splice_string_(fdt, slen + 1);
 	if (err)
 		return err;
 
 	if (!can_assume(NO_ROLLBACK))
 		*allocated = 1;
 
-	memcpy(new, s, len);
+	memcpy(new, s, slen);
+	new[slen] = '\0';
+
 	return (new - strtab);
 }
 
@@ -182,12 +184,14 @@ int fdt_del_mem_rsv(void *fdt, int n)
 }
 
 static int fdt_resize_property_(void *fdt, int nodeoffset, const char *name,
-				int len, struct fdt_property **prop)
+				int namelen, int len,
+				struct fdt_property **prop)
 {
 	int oldlen;
 	int err;
 
-	*prop = fdt_get_property_w(fdt, nodeoffset, name, &oldlen);
+	*prop = fdt_get_property_namelen_w(fdt, nodeoffset, name, namelen,
+					   &oldlen);
 	if (!*prop)
 		return oldlen;
 
@@ -200,7 +204,7 @@ static int fdt_resize_property_(void *fdt, int nodeoffset, const char *name,
 }
 
 static int fdt_add_property_(void *fdt, int nodeoffset, const char *name,
-			     int len, struct fdt_property **prop)
+			     int namelen, int len, struct fdt_property **prop)
 {
 	int proplen;
 	int nextoffset;
@@ -211,7 +215,7 @@ static int fdt_add_property_(void *fdt, int nodeoffset, const char *name,
 	if ((nextoffset = fdt_check_node_offset_(fdt, nodeoffset)) < 0)
 		return nextoffset;
 
-	namestroff = fdt_find_add_string_(fdt, name, &allocated);
+	namestroff = fdt_find_add_string_(fdt, name, namelen, &allocated);
 	if (namestroff < 0)
 		return namestroff;
 
@@ -255,17 +259,18 @@ int fdt_set_name(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)
+int fdt_setprop_namelen_placeholder(void *fdt, int nodeoffset, const char *name,
+				    int namelen, int len, void **prop_data)
 {
 	struct fdt_property *prop;
 	int err;
 
 	FDT_RW_PROBE(fdt);
 
-	err = fdt_resize_property_(fdt, nodeoffset, name, len, &prop);
+	err = fdt_resize_property_(fdt, nodeoffset, name, namelen, len, &prop);
 	if (err == -FDT_ERR_NOTFOUND)
-		err = fdt_add_property_(fdt, nodeoffset, name, len, &prop);
+		err = fdt_add_property_(fdt, nodeoffset, name, namelen, len,
+					&prop);
 	if (err)
 		return err;
 
@@ -273,13 +278,14 @@ int fdt_setprop_placeholder(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_namelen(void *fdt, int nodeoffset, const char *name,
+			int namelen, const void *val, int len)
 {
 	void *prop_data;
 	int err;
 
-	err = fdt_setprop_placeholder(fdt, nodeoffset, name, len, &prop_data);
+	err = fdt_setprop_namelen_placeholder(fdt, nodeoffset, name, namelen,
+					      len, &prop_data);
 	if (err)
 		return err;
 
@@ -307,7 +313,8 @@ int fdt_appendprop(void *fdt, int nodeoffset, const char *name,
 		prop->len = cpu_to_fdt32(newlen);
 		memcpy(prop->data + oldlen, val, len);
 	} else {
-		err = fdt_add_property_(fdt, nodeoffset, name, len, &prop);
+		err = fdt_add_property_(fdt, nodeoffset, name, strlen(name),
+					len, &prop);
 		if (err)
 			return err;
 		memcpy(prop->data, val, len);
diff --git a/libfdt/libfdt.h b/libfdt/libfdt.h
index 7d0c252a34970f4ca841935f8088410a3970127a..c9c81f039365b6d244932786a84a05f065e67c33 100644
--- a/libfdt/libfdt.h
+++ b/libfdt/libfdt.h
@@ -1666,6 +1666,38 @@ int fdt_del_mem_rsv(void *fdt, int n);
  */
 int fdt_set_name(void *fdt, int nodeoffset, const char *name);
 
+/**
+ * fdt_setprop_namelen - create or change 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
+ * @namelen: length of the name
+ * @val: pointer to data to set the property value to
+ * @len: length of the property value
+ *
+ * fdt_setprop_namelen() sets the value of the named property in the given
+ * node to the given value and length, creating the property if it
+ * does not already exist.
+ *
+ * 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_namelen(void *fdt, int nodeoffset, const char *name,
+			int namelen, const void *val, int len);
+
 /**
  * fdt_setprop - create or change a property
  * @fdt: pointer to the device tree blob
@@ -1694,8 +1726,44 @@ int fdt_set_name(void *fdt, int nodeoffset, const char *name);
  *	-FDT_ERR_BADLAYOUT,
  *	-FDT_ERR_TRUNCATED, standard meanings
  */
-int fdt_setprop(void *fdt, int nodeoffset, const char *name,
-		const void *val, int len);
+static inline int fdt_setprop(void *fdt, int nodeoffset, const char *name,
+			      const void *val, int len)
+{
+	return fdt_setprop_namelen(fdt, nodeoffset, name, strlen(name), val,
+				   len);
+}
+
+/**
+ * fdt_setprop_namelen_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
+ * @namelen: length of the name
+ * @len: length of the property value
+ * @prop_data: return pointer to property data
+ *
+ * fdt_setprop_namelen_placeholder() 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_namelen_placeholder(void *fdt, int nodeoffset, const char *name,
+				    int namelen, int len, void **prop_data);
 
 /**
  * fdt_setprop_placeholder - allocate space for a property
@@ -1725,8 +1793,13 @@ int fdt_setprop(void *fdt, int nodeoffset, const char *name,
  *	-FDT_ERR_BADLAYOUT,
  *	-FDT_ERR_TRUNCATED, standard meanings
  */
-int fdt_setprop_placeholder(void *fdt, int nodeoffset, const char *name,
-			    int len, void **prop_data);
+static inline int fdt_setprop_placeholder(void *fdt, int nodeoffset,
+					  const char *name, int len,
+					  void **prop_data)
+{
+	return fdt_setprop_namelen_placeholder(fdt, nodeoffset, name,
+					       strlen(name), len, prop_data);
+}
 
 /**
  * fdt_setprop_u32 - set a property to a 32-bit integer
diff --git a/libfdt/version.lds b/libfdt/version.lds
index 989cd89f1051ce59255a3b3e60493be4e5e985cc..76ba8f6758cef16efbad2813464e824de594b646 100644
--- a/libfdt/version.lds
+++ b/libfdt/version.lds
@@ -43,6 +43,7 @@ LIBFDT_1.2 {
 		fdt_add_mem_rsv;
 		fdt_del_mem_rsv;
 		fdt_set_name;
+		fdt_setprop_namelen;
 		fdt_setprop;
 		fdt_delprop;
 		fdt_add_subnode_namelen;
@@ -71,6 +72,7 @@ LIBFDT_1.2 {
 		fdt_find_max_phandle;
 		fdt_generate_phandle;
 		fdt_check_full;
+		fdt_setprop_namelen_placeholder;
 		fdt_setprop_placeholder;
 		fdt_property_placeholder;
 		fdt_header_size_;

-- 
2.47.1

[PATCH 4/4] libfdt: Add fdt_setprop_namelen_string()

[Ayush Singh]

Macro using fdt_setprop_namelen() internally similar to
`fdt_setprop_string()`.

Signed-off-by: Ayush Singh <ayush@beagleboard.org>
---
 libfdt/libfdt.h | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/libfdt/libfdt.h b/libfdt/libfdt.h
index c9c81f039365b6d244932786a84a05f065e67c33..5c79c7150d196062246319735b36a6111978ad56 100644
--- a/libfdt/libfdt.h
+++ b/libfdt/libfdt.h
@@ -1919,6 +1919,38 @@ static inline int fdt_setprop_cell(void *fdt, int nodeoffset, const char *name,
 #define fdt_setprop_string(fdt, nodeoffset, name, str) \
 	fdt_setprop((fdt), (nodeoffset), (name), (str), strlen(str)+1)
 
+/**
+ * fdt_setprop_namelen_string - set a property to a string value
+ * @fdt: pointer to the device tree blob
+ * @nodeoffset: offset of the node whose property to change
+ * @name: name of the property to change
+ * @namelen: number of characters of name to consider
+ * @str: string value for the property
+ *
+ * fdt_setprop_namelen_string() sets the value of the named property in the
+ * given node to the given string value (using the length of the
+ * string to determine the new length of the property), or creates a
+ * new property with that value if it does not already exist.
+ *
+ * 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
+ */
+#define fdt_setprop_namelen_string(fdt, nodeoffset, name, namelen, str)    \
+	fdt_setprop_namelen((fdt), (nodeoffset), (name), (namelen), (str), \
+			    strlen(str) + 1)
 
 /**
  * fdt_setprop_empty - set a property to an empty value

-- 
2.47.1

Re: [PATCH 1/4] libfdt: add fdt_get_property_namelen_w()

[David Gibson]

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

On Tue, Dec 03, 2024 at 11:59:59PM +0530, Ayush Singh wrote:
> Similar to the non-namelen variant, it is implemented in terms of
> fdt_get_property_namelen()
> 
> Signed-off-by: Ayush Singh <ayush@beagleboard.org>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> ---
>  libfdt/libfdt.h | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/libfdt/libfdt.h b/libfdt/libfdt.h
> index 2e182ea3314d44f30e5362ad1ca62431c0bf663b..7d0c252a34970f4ca841935f8088410a3970127a 100644
> --- a/libfdt/libfdt.h
> +++ b/libfdt/libfdt.h
> @@ -712,6 +712,13 @@ const struct fdt_property *fdt_get_property_namelen(const void *fdt,
>  						    int nodeoffset,
>  						    const char *name,
>  						    int namelen, int *lenp);
> +static inline struct fdt_property *
> +fdt_get_property_namelen_w(void *fdt, int nodeoffset, const char *name,
> +			   int namelen, int *lenp)
> +{
> +	return (struct fdt_property *)(uintptr_t)fdt_get_property_namelen(
> +		fdt, nodeoffset, name, namelen, lenp);
> +}
>  #endif
>  
>  /**
> 

-- 
David Gibson (he or they)	| 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 --]

Re: [PATCH 2/4] libfdt_internal: fdt_find_string_len_()

[David Gibson]

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

On Wed, Dec 04, 2024 at 12:00:00AM +0530, Ayush Singh wrote:
> Allow specifying string length to `fdt_find_string_`.
> fdt_find_string_() now internally uses this function.
> 
> Signed-off-by: Ayush Singh <ayush@beagleboard.org>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> ---
>  libfdt/fdt.c             |  8 ++++----
>  libfdt/libfdt_internal.h | 11 ++++++++++-
>  2 files changed, 14 insertions(+), 5 deletions(-)
> 
> diff --git a/libfdt/fdt.c b/libfdt/fdt.c
> index 20c6415b9ced113d9fcce25b69007a6b0f9e68e7..95f644c31f9431eb597b6f683741ff9be375cebf 100644
> --- a/libfdt/fdt.c
> +++ b/libfdt/fdt.c
> @@ -312,14 +312,14 @@ int fdt_next_subnode(const void *fdt, int offset)
>  	return offset;
>  }
>  
> -const char *fdt_find_string_(const char *strtab, int tabsize, const char *s)
> +const char *fdt_find_string_len_(const char *strtab, int tabsize, const char *s,
> +				 int slen)
>  {
> -	int len = strlen(s) + 1;
> -	const char *last = strtab + tabsize - len;
> +	const char *last = strtab + tabsize - (slen + 1);
>  	const char *p;
>  
>  	for (p = strtab; p <= last; p++)
> -		if (memcmp(p, s, len) == 0)
> +		if (memcmp(p, s, slen) == 0 && p[slen] == '\0')
>  			return p;
>  	return NULL;
>  }
> diff --git a/libfdt/libfdt_internal.h b/libfdt/libfdt_internal.h
> index 16bda1906a7b335519b9f748d1be6110de551e79..1fd35f364e9af15df19ae5b8b7231584804e96a4 100644
> --- a/libfdt/libfdt_internal.h
> +++ b/libfdt/libfdt_internal.h
> @@ -6,6 +6,7 @@
>   * Copyright (C) 2006 David Gibson, IBM Corporation.
>   */
>  #include <fdt.h>
> +#include <string.h>
>  
>  #define FDT_ALIGN(x, a)		(((x) + (a) - 1) & ~((a) - 1))
>  #define FDT_TAGALIGN(x)		(FDT_ALIGN((x), FDT_TAGSIZE))
> @@ -20,7 +21,15 @@ int32_t fdt_ro_probe_(const void *fdt);
>  
>  int fdt_check_node_offset_(const void *fdt, int offset);
>  int fdt_check_prop_offset_(const void *fdt, int offset);
> -const char *fdt_find_string_(const char *strtab, int tabsize, const char *s);
> +
> +const char *fdt_find_string_len_(const char *strtab, int tabsize, const char *s,
> +				 int s_len);
> +static inline const char *fdt_find_string_(const char *strtab, int tabsize,
> +					   const char *s)
> +{
> +	return fdt_find_string_len_(strtab, tabsize, s, strlen(s));
> +}
> +
>  int fdt_node_end_offset_(void *fdt, int nodeoffset);
>  
>  static inline const void *fdt_offset_ptr_(const void *fdt, int offset)
> 

-- 
David Gibson (he or they)	| 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 --]

Re: [PATCH 3/4] libfdt: Add fdt_setprop_namelen()

[David Gibson]

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

On Wed, Dec 04, 2024 at 12:00:01AM +0530, Ayush Singh wrote:
> Allow specifying name length in setprop similar to
> `fdt_get_property_namelen` functions.
> 
> Signed-off-by: Ayush Singh <ayush@beagleboard.org>

Two really minor revisions suggested below, otherwise this looks good.

> ---
>  libfdt/fdt_rw.c    | 41 +++++++++++++++------------
>  libfdt/libfdt.h    | 81 +++++++++++++++++++++++++++++++++++++++++++++++++++---
>  libfdt/version.lds |  2 ++
>  3 files changed, 103 insertions(+), 21 deletions(-)
> 
> diff --git a/libfdt/fdt_rw.c b/libfdt/fdt_rw.c
> index 3621d3651d3f4bd82b7af66c60d023e3139add03..33c60063cd721a147a3c6c0e70450c98d8dbd772 100644
> --- a/libfdt/fdt_rw.c
> +++ b/libfdt/fdt_rw.c
> @@ -124,31 +124,33 @@ static int fdt_splice_string_(void *fdt, int newlen)
>   *	allocated. Ignored if can_assume(NO_ROLLBACK)
>   * @return offset of string in the string table (whether found or added)
>   */
> -static int fdt_find_add_string_(void *fdt, const char *s, int *allocated)
> +static int fdt_find_add_string_(void *fdt, const char *s, int slen,
> +				int *allocated)
>  {
>  	char *strtab = (char *)fdt + fdt_off_dt_strings(fdt);
>  	const char *p;
>  	char *new;
> -	int len = strlen(s) + 1;
>  	int err;
>  
>  	if (!can_assume(NO_ROLLBACK))
>  		*allocated = 0;
>  
> -	p = fdt_find_string_(strtab, fdt_size_dt_strings(fdt), s);
> +	p = fdt_find_string_len_(strtab, fdt_size_dt_strings(fdt), s, slen);
>  	if (p)
>  		/* found it */
>  		return (p - strtab);
>  
>  	new = strtab + fdt_size_dt_strings(fdt);
> -	err = fdt_splice_string_(fdt, len);
> +	err = fdt_splice_string_(fdt, slen + 1);
>  	if (err)
>  		return err;
>  
>  	if (!can_assume(NO_ROLLBACK))
>  		*allocated = 1;
>  
> -	memcpy(new, s, len);
> +	memcpy(new, s, slen);
> +	new[slen] = '\0';
> +
>  	return (new - strtab);
>  }
>  
> @@ -182,12 +184,14 @@ int fdt_del_mem_rsv(void *fdt, int n)
>  }
>  
>  static int fdt_resize_property_(void *fdt, int nodeoffset, const char *name,
> -				int len, struct fdt_property **prop)
> +				int namelen, int len,
> +				struct fdt_property **prop)

Nit: I'd prefer to see name and namelen grouped together on a line,
then len and prop can go on the next.

>  {
>  	int oldlen;
>  	int err;
>  
> -	*prop = fdt_get_property_w(fdt, nodeoffset, name, &oldlen);
> +	*prop = fdt_get_property_namelen_w(fdt, nodeoffset, name, namelen,
> +					   &oldlen);
>  	if (!*prop)
>  		return oldlen;
>  
> @@ -200,7 +204,7 @@ static int fdt_resize_property_(void *fdt, int nodeoffset, const char *name,
>  }
>  
>  static int fdt_add_property_(void *fdt, int nodeoffset, const char *name,
> -			     int len, struct fdt_property **prop)
> +			     int namelen, int len, struct fdt_property **prop)
>  {
>  	int proplen;
>  	int nextoffset;
> @@ -211,7 +215,7 @@ static int fdt_add_property_(void *fdt, int nodeoffset, const char *name,
>  	if ((nextoffset = fdt_check_node_offset_(fdt, nodeoffset)) < 0)
>  		return nextoffset;
>  
> -	namestroff = fdt_find_add_string_(fdt, name, &allocated);
> +	namestroff = fdt_find_add_string_(fdt, name, namelen, &allocated);
>  	if (namestroff < 0)
>  		return namestroff;
>  
> @@ -255,17 +259,18 @@ int fdt_set_name(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)
> +int fdt_setprop_namelen_placeholder(void *fdt, int nodeoffset, const char *name,
> +				    int namelen, int len, void **prop_data)

Sorry, I gave you bad advice in my earlier message.  I'd prefer this
to be "fdt_setprop_placeholder_namelen()" (compare
"fdt_add_subnode_namelen()").

>  {
>  	struct fdt_property *prop;
>  	int err;
>  
>  	FDT_RW_PROBE(fdt);
>  
> -	err = fdt_resize_property_(fdt, nodeoffset, name, len, &prop);
> +	err = fdt_resize_property_(fdt, nodeoffset, name, namelen, len, &prop);
>  	if (err == -FDT_ERR_NOTFOUND)
> -		err = fdt_add_property_(fdt, nodeoffset, name, len, &prop);
> +		err = fdt_add_property_(fdt, nodeoffset, name, namelen, len,
> +					&prop);
>  	if (err)
>  		return err;
>  
> @@ -273,13 +278,14 @@ int fdt_setprop_placeholder(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_namelen(void *fdt, int nodeoffset, const char *name,
> +			int namelen, const void *val, int len)
>  {
>  	void *prop_data;
>  	int err;
>  
> -	err = fdt_setprop_placeholder(fdt, nodeoffset, name, len, &prop_data);
> +	err = fdt_setprop_namelen_placeholder(fdt, nodeoffset, name, namelen,
> +					      len, &prop_data);
>  	if (err)
>  		return err;
>  
> @@ -307,7 +313,8 @@ int fdt_appendprop(void *fdt, int nodeoffset, const char *name,
>  		prop->len = cpu_to_fdt32(newlen);
>  		memcpy(prop->data + oldlen, val, len);
>  	} else {
> -		err = fdt_add_property_(fdt, nodeoffset, name, len, &prop);
> +		err = fdt_add_property_(fdt, nodeoffset, name, strlen(name),
> +					len, &prop);
>  		if (err)
>  			return err;
>  		memcpy(prop->data, val, len);
> diff --git a/libfdt/libfdt.h b/libfdt/libfdt.h
> index 7d0c252a34970f4ca841935f8088410a3970127a..c9c81f039365b6d244932786a84a05f065e67c33 100644
> --- a/libfdt/libfdt.h
> +++ b/libfdt/libfdt.h
> @@ -1666,6 +1666,38 @@ int fdt_del_mem_rsv(void *fdt, int n);
>   */
>  int fdt_set_name(void *fdt, int nodeoffset, const char *name);
>  
> +/**
> + * fdt_setprop_namelen - create or change 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
> + * @namelen: length of the name
> + * @val: pointer to data to set the property value to
> + * @len: length of the property value
> + *
> + * fdt_setprop_namelen() sets the value of the named property in the given
> + * node to the given value and length, creating the property if it
> + * does not already exist.
> + *
> + * 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_namelen(void *fdt, int nodeoffset, const char *name,
> +			int namelen, const void *val, int len);
> +
>  /**
>   * fdt_setprop - create or change a property
>   * @fdt: pointer to the device tree blob
> @@ -1694,8 +1726,44 @@ int fdt_set_name(void *fdt, int nodeoffset, const char *name);
>   *	-FDT_ERR_BADLAYOUT,
>   *	-FDT_ERR_TRUNCATED, standard meanings
>   */
> -int fdt_setprop(void *fdt, int nodeoffset, const char *name,
> -		const void *val, int len);
> +static inline int fdt_setprop(void *fdt, int nodeoffset, const char *name,
> +			      const void *val, int len)
> +{
> +	return fdt_setprop_namelen(fdt, nodeoffset, name, strlen(name), val,
> +				   len);
> +}
> +
> +/**
> + * fdt_setprop_namelen_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
> + * @namelen: length of the name
> + * @len: length of the property value
> + * @prop_data: return pointer to property data
> + *
> + * fdt_setprop_namelen_placeholder() 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_namelen_placeholder(void *fdt, int nodeoffset, const char *name,
> +				    int namelen, int len, void **prop_data);
>  
>  /**
>   * fdt_setprop_placeholder - allocate space for a property
> @@ -1725,8 +1793,13 @@ int fdt_setprop(void *fdt, int nodeoffset, const char *name,
>   *	-FDT_ERR_BADLAYOUT,
>   *	-FDT_ERR_TRUNCATED, standard meanings
>   */
> -int fdt_setprop_placeholder(void *fdt, int nodeoffset, const char *name,
> -			    int len, void **prop_data);
> +static inline int fdt_setprop_placeholder(void *fdt, int nodeoffset,
> +					  const char *name, int len,
> +					  void **prop_data)
> +{
> +	return fdt_setprop_namelen_placeholder(fdt, nodeoffset, name,
> +					       strlen(name), len, prop_data);
> +}
>  
>  /**
>   * fdt_setprop_u32 - set a property to a 32-bit integer
> diff --git a/libfdt/version.lds b/libfdt/version.lds
> index 989cd89f1051ce59255a3b3e60493be4e5e985cc..76ba8f6758cef16efbad2813464e824de594b646 100644
> --- a/libfdt/version.lds
> +++ b/libfdt/version.lds
> @@ -43,6 +43,7 @@ LIBFDT_1.2 {
>  		fdt_add_mem_rsv;
>  		fdt_del_mem_rsv;
>  		fdt_set_name;
> +		fdt_setprop_namelen;
>  		fdt_setprop;
>  		fdt_delprop;
>  		fdt_add_subnode_namelen;
> @@ -71,6 +72,7 @@ LIBFDT_1.2 {
>  		fdt_find_max_phandle;
>  		fdt_generate_phandle;
>  		fdt_check_full;
> +		fdt_setprop_namelen_placeholder;
>  		fdt_setprop_placeholder;
>  		fdt_property_placeholder;
>  		fdt_header_size_;
> 

-- 
David Gibson (he or they)	| 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 --]

Re: [PATCH 4/4] libfdt: Add fdt_setprop_namelen_string()

[David Gibson]

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

On Wed, Dec 04, 2024 at 12:00:02AM +0530, Ayush Singh wrote:
> Macro using fdt_setprop_namelen() internally similar to
> `fdt_setprop_string()`.
> 
> Signed-off-by: Ayush Singh <ayush@beagleboard.org>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> ---
>  libfdt/libfdt.h | 32 ++++++++++++++++++++++++++++++++
>  1 file changed, 32 insertions(+)
> 
> diff --git a/libfdt/libfdt.h b/libfdt/libfdt.h
> index c9c81f039365b6d244932786a84a05f065e67c33..5c79c7150d196062246319735b36a6111978ad56 100644
> --- a/libfdt/libfdt.h
> +++ b/libfdt/libfdt.h
> @@ -1919,6 +1919,38 @@ static inline int fdt_setprop_cell(void *fdt, int nodeoffset, const char *name,
>  #define fdt_setprop_string(fdt, nodeoffset, name, str) \
>  	fdt_setprop((fdt), (nodeoffset), (name), (str), strlen(str)+1)
>  
> +/**
> + * fdt_setprop_namelen_string - set a property to a string value
> + * @fdt: pointer to the device tree blob
> + * @nodeoffset: offset of the node whose property to change
> + * @name: name of the property to change
> + * @namelen: number of characters of name to consider
> + * @str: string value for the property
> + *
> + * fdt_setprop_namelen_string() sets the value of the named property in the
> + * given node to the given string value (using the length of the
> + * string to determine the new length of the property), or creates a
> + * new property with that value if it does not already exist.
> + *
> + * 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
> + */
> +#define fdt_setprop_namelen_string(fdt, nodeoffset, name, namelen, str)    \
> +	fdt_setprop_namelen((fdt), (nodeoffset), (name), (namelen), (str), \
> +			    strlen(str) + 1)
>  
>  /**
>   * fdt_setprop_empty - set a property to an empty value
> 

-- 
David Gibson (he or they)	| 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 --]

Re: [PATCH 0/4] libfdt: Add namelen variants for setprop

[David Gibson]

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

On Tue, Dec 03, 2024 at 11:59:58PM +0530, Ayush Singh wrote:
> Helper functions to setproperty with length of property name similar to
> getprop_namelen variants.
> 
> This patch series was originally part of overlay-path patch series [0].
> 
> [0]: https://lore.kernel.org/devicetree-compiler/6b2dba90-3c52-4933-88f3-b47f96dc7710@beagleboard.org/T/#t
> 
> Signed-off-by: Ayush Singh <ayush@beagleboard.org>

Thanks for responding to the earlier review so quickly.  There are a
couple of tiny things to fix in 3/4, otherwise this looks good to go.

> 
> ---
> Ayush Singh (4):
>       libfdt: add fdt_get_property_namelen_w()
>       libfdt_internal: fdt_find_string_len_()
>       libfdt: Add fdt_setprop_namelen()
>       libfdt: Add fdt_setprop_namelen_string()
> 
>  libfdt/fdt.c             |   8 ++--
>  libfdt/fdt_rw.c          |  41 +++++++++-------
>  libfdt/libfdt.h          | 120 +++++++++++++++++++++++++++++++++++++++++++++--
>  libfdt/libfdt_internal.h |  11 ++++-
>  libfdt/version.lds       |   2 +
>  5 files changed, 156 insertions(+), 26 deletions(-)
> ---
> base-commit: 6f183c7d9246bde8f05d2edbc31fdd497c4a8702
> change-id: 20241203-setprop-namelen-a94f7f057acf
> 
> Best regards,

-- 
David Gibson (he or they)	| 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 --]

Re: [PATCH 3/4] libfdt: Add fdt_setprop_namelen()

[Ayush Singh]

On 04/12/24 05:59, David Gibson wrote:
> On Wed, Dec 04, 2024 at 12:00:01AM +0530, Ayush Singh wrote:
>> Allow specifying name length in setprop similar to
>> `fdt_get_property_namelen` functions.
>>
>> Signed-off-by: Ayush Singh <ayush@beagleboard.org>
> 
> Two really minor revisions suggested below, otherwise this looks good.
> 
>> ---
>>   libfdt/fdt_rw.c    | 41 +++++++++++++++------------
>>   libfdt/libfdt.h    | 81 +++++++++++++++++++++++++++++++++++++++++++++++++++---
>>   libfdt/version.lds |  2 ++
>>   3 files changed, 103 insertions(+), 21 deletions(-)
>>
>> diff --git a/libfdt/fdt_rw.c b/libfdt/fdt_rw.c
>> index 3621d3651d3f4bd82b7af66c60d023e3139add03..33c60063cd721a147a3c6c0e70450c98d8dbd772 100644
>> --- a/libfdt/fdt_rw.c
>> +++ b/libfdt/fdt_rw.c
>> @@ -124,31 +124,33 @@ static int fdt_splice_string_(void *fdt, int newlen)
>>    *	allocated. Ignored if can_assume(NO_ROLLBACK)
>>    * @return offset of string in the string table (whether found or added)
>>    */
>> -static int fdt_find_add_string_(void *fdt, const char *s, int *allocated)
>> +static int fdt_find_add_string_(void *fdt, const char *s, int slen,
>> +				int *allocated)
>>   {
>>   	char *strtab = (char *)fdt + fdt_off_dt_strings(fdt);
>>   	const char *p;
>>   	char *new;
>> -	int len = strlen(s) + 1;
>>   	int err;
>>   
>>   	if (!can_assume(NO_ROLLBACK))
>>   		*allocated = 0;
>>   
>> -	p = fdt_find_string_(strtab, fdt_size_dt_strings(fdt), s);
>> +	p = fdt_find_string_len_(strtab, fdt_size_dt_strings(fdt), s, slen);
>>   	if (p)
>>   		/* found it */
>>   		return (p - strtab);
>>   
>>   	new = strtab + fdt_size_dt_strings(fdt);
>> -	err = fdt_splice_string_(fdt, len);
>> +	err = fdt_splice_string_(fdt, slen + 1);
>>   	if (err)
>>   		return err;
>>   
>>   	if (!can_assume(NO_ROLLBACK))
>>   		*allocated = 1;
>>   
>> -	memcpy(new, s, len);
>> +	memcpy(new, s, slen);
>> +	new[slen] = '\0';
>> +
>>   	return (new - strtab);
>>   }
>>   
>> @@ -182,12 +184,14 @@ int fdt_del_mem_rsv(void *fdt, int n)
>>   }
>>   
>>   static int fdt_resize_property_(void *fdt, int nodeoffset, const char *name,
>> -				int len, struct fdt_property **prop)
>> +				int namelen, int len,
>> +				struct fdt_property **prop)
> 
> Nit: I'd prefer to see name and namelen grouped together on a line,
> then len and prop can go on the next.

That exceeds 80 column limit. I am fine with 100 column limit to be 
honest, but well, I would like that limit to apply everywhere (in any 
new changes), instead of just here.

> 
>>   {
>>   	int oldlen;
>>   	int err;
>>   
>> -	*prop = fdt_get_property_w(fdt, nodeoffset, name, &oldlen);
>> +	*prop = fdt_get_property_namelen_w(fdt, nodeoffset, name, namelen,
>> +					   &oldlen);
>>   	if (!*prop)
>>   		return oldlen;
>>   
>> @@ -200,7 +204,7 @@ static int fdt_resize_property_(void *fdt, int nodeoffset, const char *name,
>>   }
>>   
>>   static int fdt_add_property_(void *fdt, int nodeoffset, const char *name,
>> -			     int len, struct fdt_property **prop)
>> +			     int namelen, int len, struct fdt_property **prop)
>>   {
>>   	int proplen;
>>   	int nextoffset;
>> @@ -211,7 +215,7 @@ static int fdt_add_property_(void *fdt, int nodeoffset, const char *name,
>>   	if ((nextoffset = fdt_check_node_offset_(fdt, nodeoffset)) < 0)
>>   		return nextoffset;
>>   
>> -	namestroff = fdt_find_add_string_(fdt, name, &allocated);
>> +	namestroff = fdt_find_add_string_(fdt, name, namelen, &allocated);
>>   	if (namestroff < 0)
>>   		return namestroff;
>>   
>> @@ -255,17 +259,18 @@ int fdt_set_name(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)
>> +int fdt_setprop_namelen_placeholder(void *fdt, int nodeoffset, const char *name,
>> +				    int namelen, int len, void **prop_data)
> 
> Sorry, I gave you bad advice in my earlier message.  I'd prefer this
> to be "fdt_setprop_placeholder_namelen()" (compare
> "fdt_add_subnode_namelen()").
> 
>>   {
>>   	struct fdt_property *prop;
>>   	int err;
>>   
>>   	FDT_RW_PROBE(fdt);
>>   
>> -	err = fdt_resize_property_(fdt, nodeoffset, name, len, &prop);
>> +	err = fdt_resize_property_(fdt, nodeoffset, name, namelen, len, &prop);
>>   	if (err == -FDT_ERR_NOTFOUND)
>> -		err = fdt_add_property_(fdt, nodeoffset, name, len, &prop);
>> +		err = fdt_add_property_(fdt, nodeoffset, name, namelen, len,
>> +					&prop);
>>   	if (err)
>>   		return err;
>>   
>> @@ -273,13 +278,14 @@ int fdt_setprop_placeholder(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_namelen(void *fdt, int nodeoffset, const char *name,
>> +			int namelen, const void *val, int len)
>>   {
>>   	void *prop_data;
>>   	int err;
>>   
>> -	err = fdt_setprop_placeholder(fdt, nodeoffset, name, len, &prop_data);
>> +	err = fdt_setprop_namelen_placeholder(fdt, nodeoffset, name, namelen,
>> +					      len, &prop_data);
>>   	if (err)
>>   		return err;
>>   
>> @@ -307,7 +313,8 @@ int fdt_appendprop(void *fdt, int nodeoffset, const char *name,
>>   		prop->len = cpu_to_fdt32(newlen);
>>   		memcpy(prop->data + oldlen, val, len);
>>   	} else {
>> -		err = fdt_add_property_(fdt, nodeoffset, name, len, &prop);
>> +		err = fdt_add_property_(fdt, nodeoffset, name, strlen(name),
>> +					len, &prop);
>>   		if (err)
>>   			return err;
>>   		memcpy(prop->data, val, len);
>> diff --git a/libfdt/libfdt.h b/libfdt/libfdt.h
>> index 7d0c252a34970f4ca841935f8088410a3970127a..c9c81f039365b6d244932786a84a05f065e67c33 100644
>> --- a/libfdt/libfdt.h
>> +++ b/libfdt/libfdt.h
>> @@ -1666,6 +1666,38 @@ int fdt_del_mem_rsv(void *fdt, int n);
>>    */
>>   int fdt_set_name(void *fdt, int nodeoffset, const char *name);
>>   
>> +/**
>> + * fdt_setprop_namelen - create or change 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
>> + * @namelen: length of the name
>> + * @val: pointer to data to set the property value to
>> + * @len: length of the property value
>> + *
>> + * fdt_setprop_namelen() sets the value of the named property in the given
>> + * node to the given value and length, creating the property if it
>> + * does not already exist.
>> + *
>> + * 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_namelen(void *fdt, int nodeoffset, const char *name,
>> +			int namelen, const void *val, int len);
>> +
>>   /**
>>    * fdt_setprop - create or change a property
>>    * @fdt: pointer to the device tree blob
>> @@ -1694,8 +1726,44 @@ int fdt_set_name(void *fdt, int nodeoffset, const char *name);
>>    *	-FDT_ERR_BADLAYOUT,
>>    *	-FDT_ERR_TRUNCATED, standard meanings
>>    */
>> -int fdt_setprop(void *fdt, int nodeoffset, const char *name,
>> -		const void *val, int len);
>> +static inline int fdt_setprop(void *fdt, int nodeoffset, const char *name,
>> +			      const void *val, int len)
>> +{
>> +	return fdt_setprop_namelen(fdt, nodeoffset, name, strlen(name), val,
>> +				   len);
>> +}
>> +
>> +/**
>> + * fdt_setprop_namelen_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
>> + * @namelen: length of the name
>> + * @len: length of the property value
>> + * @prop_data: return pointer to property data
>> + *
>> + * fdt_setprop_namelen_placeholder() 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_namelen_placeholder(void *fdt, int nodeoffset, const char *name,
>> +				    int namelen, int len, void **prop_data);
>>   
>>   /**
>>    * fdt_setprop_placeholder - allocate space for a property
>> @@ -1725,8 +1793,13 @@ int fdt_setprop(void *fdt, int nodeoffset, const char *name,
>>    *	-FDT_ERR_BADLAYOUT,
>>    *	-FDT_ERR_TRUNCATED, standard meanings
>>    */
>> -int fdt_setprop_placeholder(void *fdt, int nodeoffset, const char *name,
>> -			    int len, void **prop_data);
>> +static inline int fdt_setprop_placeholder(void *fdt, int nodeoffset,
>> +					  const char *name, int len,
>> +					  void **prop_data)
>> +{
>> +	return fdt_setprop_namelen_placeholder(fdt, nodeoffset, name,
>> +					       strlen(name), len, prop_data);
>> +}
>>   
>>   /**
>>    * fdt_setprop_u32 - set a property to a 32-bit integer
>> diff --git a/libfdt/version.lds b/libfdt/version.lds
>> index 989cd89f1051ce59255a3b3e60493be4e5e985cc..76ba8f6758cef16efbad2813464e824de594b646 100644
>> --- a/libfdt/version.lds
>> +++ b/libfdt/version.lds
>> @@ -43,6 +43,7 @@ LIBFDT_1.2 {
>>   		fdt_add_mem_rsv;
>>   		fdt_del_mem_rsv;
>>   		fdt_set_name;
>> +		fdt_setprop_namelen;
>>   		fdt_setprop;
>>   		fdt_delprop;
>>   		fdt_add_subnode_namelen;
>> @@ -71,6 +72,7 @@ LIBFDT_1.2 {
>>   		fdt_find_max_phandle;
>>   		fdt_generate_phandle;
>>   		fdt_check_full;
>> +		fdt_setprop_namelen_placeholder;
>>   		fdt_setprop_placeholder;
>>   		fdt_property_placeholder;
>>   		fdt_header_size_;
>>
> 

Ayush Singh

Re: [PATCH 3/4] libfdt: Add fdt_setprop_namelen()

[David Gibson]

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

On Wed, Dec 04, 2024 at 01:09:13PM +0530, Ayush Singh wrote:
> 
> 
> On 04/12/24 05:59, David Gibson wrote:
> > On Wed, Dec 04, 2024 at 12:00:01AM +0530, Ayush Singh wrote:
> > > Allow specifying name length in setprop similar to
> > > `fdt_get_property_namelen` functions.
> > > 
> > > Signed-off-by: Ayush Singh <ayush@beagleboard.org>
> > 
> > Two really minor revisions suggested below, otherwise this looks good.
> > 
> > > ---
> > >   libfdt/fdt_rw.c    | 41 +++++++++++++++------------
> > >   libfdt/libfdt.h    | 81 +++++++++++++++++++++++++++++++++++++++++++++++++++---
> > >   libfdt/version.lds |  2 ++
> > >   3 files changed, 103 insertions(+), 21 deletions(-)
> > > 
> > > diff --git a/libfdt/fdt_rw.c b/libfdt/fdt_rw.c
> > > index 3621d3651d3f4bd82b7af66c60d023e3139add03..33c60063cd721a147a3c6c0e70450c98d8dbd772 100644
> > > --- a/libfdt/fdt_rw.c
> > > +++ b/libfdt/fdt_rw.c
> > > @@ -124,31 +124,33 @@ static int fdt_splice_string_(void *fdt, int newlen)
> > >    *	allocated. Ignored if can_assume(NO_ROLLBACK)
> > >    * @return offset of string in the string table (whether found or added)
> > >    */
> > > -static int fdt_find_add_string_(void *fdt, const char *s, int *allocated)
> > > +static int fdt_find_add_string_(void *fdt, const char *s, int slen,
> > > +				int *allocated)
> > >   {
> > >   	char *strtab = (char *)fdt + fdt_off_dt_strings(fdt);
> > >   	const char *p;
> > >   	char *new;
> > > -	int len = strlen(s) + 1;
> > >   	int err;
> > >   	if (!can_assume(NO_ROLLBACK))
> > >   		*allocated = 0;
> > > -	p = fdt_find_string_(strtab, fdt_size_dt_strings(fdt), s);
> > > +	p = fdt_find_string_len_(strtab, fdt_size_dt_strings(fdt), s, slen);
> > >   	if (p)
> > >   		/* found it */
> > >   		return (p - strtab);
> > >   	new = strtab + fdt_size_dt_strings(fdt);
> > > -	err = fdt_splice_string_(fdt, len);
> > > +	err = fdt_splice_string_(fdt, slen + 1);
> > >   	if (err)
> > >   		return err;
> > >   	if (!can_assume(NO_ROLLBACK))
> > >   		*allocated = 1;
> > > -	memcpy(new, s, len);
> > > +	memcpy(new, s, slen);
> > > +	new[slen] = '\0';
> > > +
> > >   	return (new - strtab);
> > >   }
> > > @@ -182,12 +184,14 @@ int fdt_del_mem_rsv(void *fdt, int n)
> > >   }
> > >   static int fdt_resize_property_(void *fdt, int nodeoffset, const char *name,
> > > -				int len, struct fdt_property **prop)
> > > +				int namelen, int len,
> > > +				struct fdt_property **prop)
> > 
> > Nit: I'd prefer to see name and namelen grouped together on a line,
> > then len and prop can go on the next.
> 
> That exceeds 80 column limit. I am fine with 100 column limit to be honest,
> but well, I would like that limit to apply everywhere (in any new changes),
> instead of just here.

Sorry, I was unclear.  I meant:

static int fdt_resize_property_(void *fdt, int nodeoffset,
                                const char *name, int namelen,
                                int len, struct fdt_property **prop)

> 
> > 
> > >   {
> > >   	int oldlen;
> > >   	int err;
> > > -	*prop = fdt_get_property_w(fdt, nodeoffset, name, &oldlen);
> > > +	*prop = fdt_get_property_namelen_w(fdt, nodeoffset, name, namelen,
> > > +					   &oldlen);
> > >   	if (!*prop)
> > >   		return oldlen;
> > > @@ -200,7 +204,7 @@ static int fdt_resize_property_(void *fdt, int nodeoffset, const char *name,
> > >   }
> > >   static int fdt_add_property_(void *fdt, int nodeoffset, const char *name,
> > > -			     int len, struct fdt_property **prop)
> > > +			     int namelen, int len, struct fdt_property **prop)
> > >   {
> > >   	int proplen;
> > >   	int nextoffset;
> > > @@ -211,7 +215,7 @@ static int fdt_add_property_(void *fdt, int nodeoffset, const char *name,
> > >   	if ((nextoffset = fdt_check_node_offset_(fdt, nodeoffset)) < 0)
> > >   		return nextoffset;
> > > -	namestroff = fdt_find_add_string_(fdt, name, &allocated);
> > > +	namestroff = fdt_find_add_string_(fdt, name, namelen, &allocated);
> > >   	if (namestroff < 0)
> > >   		return namestroff;
> > > @@ -255,17 +259,18 @@ int fdt_set_name(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)
> > > +int fdt_setprop_namelen_placeholder(void *fdt, int nodeoffset, const char *name,
> > > +				    int namelen, int len, void **prop_data)
> > 
> > Sorry, I gave you bad advice in my earlier message.  I'd prefer this
> > to be "fdt_setprop_placeholder_namelen()" (compare
> > "fdt_add_subnode_namelen()").
> > 
> > >   {
> > >   	struct fdt_property *prop;
> > >   	int err;
> > >   	FDT_RW_PROBE(fdt);
> > > -	err = fdt_resize_property_(fdt, nodeoffset, name, len, &prop);
> > > +	err = fdt_resize_property_(fdt, nodeoffset, name, namelen, len, &prop);
> > >   	if (err == -FDT_ERR_NOTFOUND)
> > > -		err = fdt_add_property_(fdt, nodeoffset, name, len, &prop);
> > > +		err = fdt_add_property_(fdt, nodeoffset, name, namelen, len,
> > > +					&prop);
> > >   	if (err)
> > >   		return err;
> > > @@ -273,13 +278,14 @@ int fdt_setprop_placeholder(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_namelen(void *fdt, int nodeoffset, const char *name,
> > > +			int namelen, const void *val, int len)
> > >   {
> > >   	void *prop_data;
> > >   	int err;
> > > -	err = fdt_setprop_placeholder(fdt, nodeoffset, name, len, &prop_data);
> > > +	err = fdt_setprop_namelen_placeholder(fdt, nodeoffset, name, namelen,
> > > +					      len, &prop_data);
> > >   	if (err)
> > >   		return err;
> > > @@ -307,7 +313,8 @@ int fdt_appendprop(void *fdt, int nodeoffset, const char *name,
> > >   		prop->len = cpu_to_fdt32(newlen);
> > >   		memcpy(prop->data + oldlen, val, len);
> > >   	} else {
> > > -		err = fdt_add_property_(fdt, nodeoffset, name, len, &prop);
> > > +		err = fdt_add_property_(fdt, nodeoffset, name, strlen(name),
> > > +					len, &prop);
> > >   		if (err)
> > >   			return err;
> > >   		memcpy(prop->data, val, len);
> > > diff --git a/libfdt/libfdt.h b/libfdt/libfdt.h
> > > index 7d0c252a34970f4ca841935f8088410a3970127a..c9c81f039365b6d244932786a84a05f065e67c33 100644
> > > --- a/libfdt/libfdt.h
> > > +++ b/libfdt/libfdt.h
> > > @@ -1666,6 +1666,38 @@ int fdt_del_mem_rsv(void *fdt, int n);
> > >    */
> > >   int fdt_set_name(void *fdt, int nodeoffset, const char *name);
> > > +/**
> > > + * fdt_setprop_namelen - create or change 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
> > > + * @namelen: length of the name
> > > + * @val: pointer to data to set the property value to
> > > + * @len: length of the property value
> > > + *
> > > + * fdt_setprop_namelen() sets the value of the named property in the given
> > > + * node to the given value and length, creating the property if it
> > > + * does not already exist.
> > > + *
> > > + * 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_namelen(void *fdt, int nodeoffset, const char *name,
> > > +			int namelen, const void *val, int len);
> > > +
> > >   /**
> > >    * fdt_setprop - create or change a property
> > >    * @fdt: pointer to the device tree blob
> > > @@ -1694,8 +1726,44 @@ int fdt_set_name(void *fdt, int nodeoffset, const char *name);
> > >    *	-FDT_ERR_BADLAYOUT,
> > >    *	-FDT_ERR_TRUNCATED, standard meanings
> > >    */
> > > -int fdt_setprop(void *fdt, int nodeoffset, const char *name,
> > > -		const void *val, int len);
> > > +static inline int fdt_setprop(void *fdt, int nodeoffset, const char *name,
> > > +			      const void *val, int len)
> > > +{
> > > +	return fdt_setprop_namelen(fdt, nodeoffset, name, strlen(name), val,
> > > +				   len);
> > > +}
> > > +
> > > +/**
> > > + * fdt_setprop_namelen_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
> > > + * @namelen: length of the name
> > > + * @len: length of the property value
> > > + * @prop_data: return pointer to property data
> > > + *
> > > + * fdt_setprop_namelen_placeholder() 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_namelen_placeholder(void *fdt, int nodeoffset, const char *name,
> > > +				    int namelen, int len, void **prop_data);
> > >   /**
> > >    * fdt_setprop_placeholder - allocate space for a property
> > > @@ -1725,8 +1793,13 @@ int fdt_setprop(void *fdt, int nodeoffset, const char *name,
> > >    *	-FDT_ERR_BADLAYOUT,
> > >    *	-FDT_ERR_TRUNCATED, standard meanings
> > >    */
> > > -int fdt_setprop_placeholder(void *fdt, int nodeoffset, const char *name,
> > > -			    int len, void **prop_data);
> > > +static inline int fdt_setprop_placeholder(void *fdt, int nodeoffset,
> > > +					  const char *name, int len,
> > > +					  void **prop_data)
> > > +{
> > > +	return fdt_setprop_namelen_placeholder(fdt, nodeoffset, name,
> > > +					       strlen(name), len, prop_data);
> > > +}
> > >   /**
> > >    * fdt_setprop_u32 - set a property to a 32-bit integer
> > > diff --git a/libfdt/version.lds b/libfdt/version.lds
> > > index 989cd89f1051ce59255a3b3e60493be4e5e985cc..76ba8f6758cef16efbad2813464e824de594b646 100644
> > > --- a/libfdt/version.lds
> > > +++ b/libfdt/version.lds
> > > @@ -43,6 +43,7 @@ LIBFDT_1.2 {
> > >   		fdt_add_mem_rsv;
> > >   		fdt_del_mem_rsv;
> > >   		fdt_set_name;
> > > +		fdt_setprop_namelen;
> > >   		fdt_setprop;
> > >   		fdt_delprop;
> > >   		fdt_add_subnode_namelen;
> > > @@ -71,6 +72,7 @@ LIBFDT_1.2 {
> > >   		fdt_find_max_phandle;
> > >   		fdt_generate_phandle;
> > >   		fdt_check_full;
> > > +		fdt_setprop_namelen_placeholder;
> > >   		fdt_setprop_placeholder;
> > >   		fdt_property_placeholder;
> > >   		fdt_header_size_;
> > > 
> > 
> 
> Ayush Singh
> 
> 

-- 
David Gibson (he or they)	| 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 --]