[RFC 0/2] Add node and property realloc functions.

[RFC 0/2] Add node and property realloc functions.

[Srinivas KANDAGATLA]

If you try to insert a new node or extend a property with large value, 
using fdtput you will notice that it always fails.

example:
fdtput -v -p -ts ./tst.dtb "/node-1" "property-1" "str10"
Error at 'node-1': FDT_ERR_NOSPACE

or

fdtput -v -c ./tst.dtb "/node-1"
Error at 'node-1': FDT_ERR_NOSPACE


fdtput -v  -ts ./tst.dtb "/node" "property" "very big value"
Decoding value:
	string: 'very big value'
Value size 15
Error at 'property': FDT_ERR_NOSPACE

All these error are returned from libfdt, as the size of the fdt passed
has not space to accomdate these new properties.

However,
libfdt has code to add new property or node or extend a property to an
arbitary value, however it cannot be used because all the library
functions take preallocated fdt pointer limted to a size.

Adding realloc function into libfdt can help tools like fdtput to insert
nodes or properties or extend a property.

As libfdt has knowlege of how much space a new property or node can take.
This patch set attempts to fix this issue by adding new libary functions
fdt_realloc_property and fdt_realloc_node.

comments?

Srinivas Kandagatla (2):
  libfdt: Add node and property realloc functions.
  fdtput: use libfdt realloc functions.

 fdtput.c        |   30 +++++++++++++++++-------------
 libfdt/fdt_rw.c |   39 +++++++++++++++++++++++++++++++++++++++
 libfdt/libfdt.h |    4 ++++
 3 files changed, 60 insertions(+), 13 deletions(-)

-- 
1.7.6.5

[RFC 1/2] libfdt: Add node and property realloc functions.

[Srinivas KANDAGATLA]

libfdt has code to add new property or node or extend a property to an
arbitary value, however it cannot be used because all the library
functions take preallocated fdt pointer limted to a size.

Adding realloc function into libfdt can help tools like fdtput to insert
nodes or properties or extend a property.

Without this patch if we try to add a new node/property to a dtb, the
libfdt errors with FDT_ERR_NOSPACE.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla-qxv4g6HH51o@public.gmane.org>
---
 libfdt/fdt_rw.c |   39 +++++++++++++++++++++++++++++++++++++++
 libfdt/libfdt.h |    4 ++++
 2 files changed, 43 insertions(+), 0 deletions(-)

diff --git a/libfdt/fdt_rw.c b/libfdt/fdt_rw.c
index fdba618..e7766d0 100644
--- a/libfdt/fdt_rw.c
+++ b/libfdt/fdt_rw.c
@@ -52,6 +52,7 @@
 
 #include <fdt.h>
 #include <libfdt.h>
+#include <stdlib.h>
 
 #include "libfdt_internal.h"
 
@@ -248,6 +249,13 @@ static int _fdt_add_property(void *fdt, int nodeoffset, const char *name,
 	return 0;
 }
 
+static int _fdt_realloc(void **fdt, int newsize)
+{
+	*fdt = realloc(*fdt, newsize);
+	fdt_set_totalsize(*fdt, newsize);
+	return 0;
+}
+
 int fdt_set_name(void *fdt, int nodeoffset, const char *name)
 {
 	char *namep;
@@ -490,3 +498,34 @@ int fdt_pack(void *fdt)
 
 	return 0;
 }
+
+int fdt_realloc_node(void **fdt, const char *name)
+{
+	int delta = 0;
+	int newlen = strlen(name);
+	if (newlen)
+		delta = sizeof(struct fdt_node_header) +
+				FDT_TAGALIGN(newlen + 1) + FDT_TAGSIZE;
+	else
+		return 0;
+
+	return _fdt_realloc(fdt, fdt_totalsize(*fdt) + delta);
+}
+
+int fdt_realloc_property(void **fdt, int nodeoffset,
+		const char *name, int newlen)
+{
+	int delta = 0;
+	int oldlen;
+	if (!fdt_get_property(*fdt, nodeoffset, name, &oldlen))
+		/* strings + property header */
+		delta = sizeof(struct fdt_property) + strlen(name) + 1;
+
+	if (newlen > oldlen)
+		/* actual value in off_struct */
+		delta += FDT_TAGALIGN(newlen) - FDT_TAGALIGN(oldlen);
+	else
+		return 0;
+
+	return _fdt_realloc(fdt, fdt_totalsize(*fdt) + delta);
+}
diff --git a/libfdt/libfdt.h b/libfdt/libfdt.h
index 130789a..640c740 100644
--- a/libfdt/libfdt.h
+++ b/libfdt/libfdt.h
@@ -1032,6 +1032,10 @@ int fdt_create_empty_tree(void *buf, int bufsize);
 int fdt_open_into(const void *fdt, void *buf, int bufsize);
 int fdt_pack(void *fdt);
 
+int fdt_realloc_property(void **fdt, int nodeoffset,
+				const char *name, int newlen);
+int fdt_realloc_node(void **fdt, const char *name);
+
 /**
  * fdt_add_mem_rsv - add one memory reserve map entry
  * @fdt: pointer to the device tree blob
-- 
1.7.6.5

Re: [RFC 1/2] libfdt: Add node and property realloc functions.

[David Gibson]

[-- Attachment #1.1: Type: text/plain, Size: 1541 bytes --]

On Wed, Apr 10, 2013 at 03:33:18PM +0100, Srinivas KANDAGATLA wrote:
> libfdt has code to add new property or node or extend a property to an
> arbitary value, however it cannot be used because all the library
> functions take preallocated fdt pointer limted to a size.
> 
> Adding realloc function into libfdt can help tools like fdtput to insert
> nodes or properties or extend a property.
> 
> Without this patch if we try to add a new node/property to a dtb, the
> libfdt errors with FDT_ERR_NOSPACE.
> 
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla-qxv4g6HH51o@public.gmane.org>
> ---
>  libfdt/fdt_rw.c |   39 +++++++++++++++++++++++++++++++++++++++
>  libfdt/libfdt.h |    4 ++++
>  2 files changed, 43 insertions(+), 0 deletions(-)
> 
> diff --git a/libfdt/fdt_rw.c b/libfdt/fdt_rw.c
> index fdba618..e7766d0 100644
> --- a/libfdt/fdt_rw.c
> +++ b/libfdt/fdt_rw.c
> @@ -52,6 +52,7 @@
>  
>  #include <fdt.h>
>  #include <libfdt.h>
> +#include <stdlib.h>

This is kind of the heart of the matter.  libfdt is supposed to be
embeddeable into bootloaders and other weird and limited environments.
For this reason we can never directly include standard libc headers.
Instead *every* dependency of libfdt must be provided by libfdt_env.h
(which can be replaced depending on your build environment).

-- 
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 #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[-- Attachment #2: Type: text/plain, Size: 192 bytes --]

_______________________________________________
devicetree-discuss mailing list
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

[RFC 2/2] fdtput: use libfdt realloc functions.

[Srinivas KANDAGATLA]

This patch makes use of realloc functions added to libfdt.
Now the tool can insert a new node or property or extend a property with
new value greater than original size.

Without this patch fdtput tool complains with FDT_ERR_NOSPACE when we
try to add a node/property or extend the value of a property.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla-qxv4g6HH51o@public.gmane.org>
---
 fdtput.c |   30 +++++++++++++++++-------------
 1 files changed, 17 insertions(+), 13 deletions(-)

diff --git a/fdtput.c b/fdtput.c
index f2197f5..8e80f37 100644
--- a/fdtput.c
+++ b/fdtput.c
@@ -131,19 +131,20 @@ static int encode_value(struct display_info *disp, char **arg, int arg_count,
 	return 0;
 }
 
-static int store_key_value(void *blob, const char *node_name,
+static int store_key_value(void **blob, const char *node_name,
 		const char *property, const char *buf, int len)
 {
 	int node;
 	int err;
 
-	node = fdt_path_offset(blob, node_name);
+	node = fdt_path_offset(*blob, node_name);
 	if (node < 0) {
 		report_error(node_name, -1, node);
 		return -1;
 	}
 
-	err = fdt_setprop(blob, node, property, buf, len);
+	fdt_realloc_property(blob, node, property, len);
+	err = fdt_setprop(*blob, node, property, buf, len);
 	if (err) {
 		report_error(property, -1, err);
 		return -1;
@@ -161,7 +162,7 @@ static int store_key_value(void *blob, const char *node_name,
  * @param in_path	Path to process
  * @return 0 if ok, -1 on error
  */
-static int create_paths(void *blob, const char *in_path)
+static int create_paths(void **blob, const char *in_path)
 {
 	const char *path = in_path;
 	const char *sep;
@@ -177,10 +178,11 @@ static int create_paths(void *blob, const char *in_path)
 		if (!sep)
 			sep = path + strlen(path);
 
-		node = fdt_subnode_offset_namelen(blob, offset, path,
+		node = fdt_subnode_offset_namelen(*blob, offset, path,
 				sep - path);
 		if (node == -FDT_ERR_NOTFOUND) {
-			node = fdt_add_subnode_namelen(blob, offset, path,
+			fdt_realloc_node(blob, path);
+			node = fdt_add_subnode_namelen(*blob, offset, path,
 						       sep - path);
 		}
 		if (node < 0) {
@@ -203,7 +205,7 @@ static int create_paths(void *blob, const char *in_path)
  * @param node_name	Name of node to create
  * @return new node offset if found, or -1 on failure
  */
-static int create_node(void *blob, const char *node_name)
+static int create_node(void **blob, const char *node_name)
 {
 	int node = 0;
 	char *p;
@@ -216,14 +218,15 @@ static int create_node(void *blob, const char *node_name)
 	*p = '\0';
 
 	if (p > node_name) {
-		node = fdt_path_offset(blob, node_name);
+		node = fdt_path_offset(*blob, node_name);
 		if (node < 0) {
 			report_error(node_name, -1, node);
 			return -1;
 		}
 	}
 
-	node = fdt_add_subnode(blob, node, p + 1);
+	fdt_realloc_node(blob, p + 1);
+	node = fdt_add_subnode(*blob, node, p + 1);
 	if (node < 0) {
 		report_error(p + 1, -1, node);
 		return -1;
@@ -250,18 +253,19 @@ static int do_fdtput(struct display_info *disp, const char *filename,
 		 * store them into the property.
 		 */
 		assert(arg_count >= 2);
-		if (disp->auto_path && create_paths(blob, *arg))
+		if (disp->auto_path && create_paths((void **)&blob, *arg))
 			return -1;
 		if (encode_value(disp, arg + 2, arg_count - 2, &value, &len) ||
-			store_key_value(blob, *arg, arg[1], value, len))
+			store_key_value((void **)&blob, *arg,
+							arg[1], value, len))
 			ret = -1;
 		break;
 	case OPER_CREATE_NODE:
 		for (; ret >= 0 && arg_count--; arg++) {
 			if (disp->auto_path)
-				ret = create_paths(blob, *arg);
+				ret = create_paths((void **)&blob, *arg);
 			else
-				ret = create_node(blob, *arg);
+				ret = create_node((void **)&blob, *arg);
 		}
 		break;
 	}
-- 
1.7.6.5

Re: [RFC 0/2] Add node and property realloc functions.

[David Gibson]

[-- Attachment #1.1: Type: text/plain, Size: 2000 bytes --]

On Wed, Apr 10, 2013 at 03:31:18PM +0100, Srinivas KANDAGATLA wrote:
> If you try to insert a new node or extend a property with large value, 
> using fdtput you will notice that it always fails.
> 
> example:
> fdtput -v -p -ts ./tst.dtb "/node-1" "property-1" "str10"
> Error at 'node-1': FDT_ERR_NOSPACE
> 
> or
> 
> fdtput -v -c ./tst.dtb "/node-1"
> Error at 'node-1': FDT_ERR_NOSPACE
> 
> 
> fdtput -v  -ts ./tst.dtb "/node" "property" "very big value"
> Decoding value:
> 	string: 'very big value'
> Value size 15
> Error at 'property': FDT_ERR_NOSPACE
> 
> All these error are returned from libfdt, as the size of the fdt passed
> has not space to accomdate these new properties.
> 
> However,
> libfdt has code to add new property or node or extend a property to an
> arbitary value, however it cannot be used because all the library
> functions take preallocated fdt pointer limted to a size.
> 
> Adding realloc function into libfdt can help tools like fdtput to insert
> nodes or properties or extend a property.

Nack.

libfdt deliberately does not use an allocator, to permit embedding in
weird and limited environments such as bootloaders.

I've thought about adding an optional layer of allocator aware
functions, but it's not quite as straightforward as you'd think
because you need to be clear about what the assumptions are on the
allocation of the given blob - there's no place to put extra context
to indicate whether it is malloc()ed or part of a larger structure,
for example.  At the very lease allocating functions would need to
move to a separate source file from the existing functions.

I think the correct fix in this case is to make fdtput handle the
FDT_ERR_NOSPACE by reallocating itself, using fdt_open_into() and
retrying.

-- 
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 #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[-- Attachment #2: Type: text/plain, Size: 192 bytes --]

_______________________________________________
devicetree-discuss mailing list
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

Re: [RFC 0/2] Add node and property realloc functions.

[Srinivas KANDAGATLA]

On 11/04/13 02:44, David Gibson wrote:
> On Wed, Apr 10, 2013 at 03:31:18PM +0100, Srinivas KANDAGATLA wrote:
>> If you try to insert a new node or extend a property with large value, 
>> using fdtput you will notice that it always fails.
>>
>> example:
>> fdtput -v -p -ts ./tst.dtb "/node-1" "property-1" "str10"
>> Error at 'node-1': FDT_ERR_NOSPACE
>>
>> or
>>
>> fdtput -v -c ./tst.dtb "/node-1"
>> Error at 'node-1': FDT_ERR_NOSPACE
>>
>>
>> fdtput -v  -ts ./tst.dtb "/node" "property" "very big value"
>> Decoding value:
>> 	string: 'very big value'
>> Value size 15
>> Error at 'property': FDT_ERR_NOSPACE
>>
>> All these error are returned from libfdt, as the size of the fdt passed
>> has not space to accomdate these new properties.
>>
>> However,
>> libfdt has code to add new property or node or extend a property to an
>> arbitary value, however it cannot be used because all the library
>> functions take preallocated fdt pointer limted to a size.
>>
>> Adding realloc function into libfdt can help tools like fdtput to insert
>> nodes or properties or extend a property.
> Nack.
>
> libfdt deliberately does not use an allocator, to permit embedding in
> weird and limited environments such as bootloaders.
>
> I've thought about adding an optional layer of allocator aware
> functions, but it's not quite as straightforward as you'd think
> because you need to be clear about what the assumptions are on the
> allocation of the given blob - there's no place to put extra context
> to indicate whether it is malloc()ed or part of a larger structure,
> for example.  At the very lease allocating functions would need to
> move to a separate source file from the existing functions.
>
> I think the correct fix in this case is to make fdtput handle the
> FDT_ERR_NOSPACE by reallocating itself, using fdt_open_into() and
> retrying.
Thanks for the comment, I will try to fix fdtput with fdt_open_into.
>
>
>
> _______________________________________________
> devicetree-discuss mailing list
> devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
> https://lists.ozlabs.org/listinfo/devicetree-discuss