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

Ayush Singh <ayush@xxxxxxxxxxxxxxx> · Wed, 4 Dec 2024 13:09:13 +0530

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@xxxxxxxxxxxxxxx>

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








