[PATCH v2] fdtput: expand fdt if value does not fit (v2).
David Gibson
dwg at au1.ibm.com
Mon Apr 29 19:08:35 EST 2013
On Fri, Apr 19, 2013 at 09:36:36AM +0100, Srinivas KANDAGATLA wrote:
> From: Srinivas Kandagatla <srinivas.kandagatla at st.com>
>
> 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" "value-1
> Error at 'node-1': FDT_ERR_NOSPACE
>
> or
>
> fdtput -v -c ./tst.dtb "/node-1"
> Error at 'node-1': FDT_ERR_NOSPACE
>
> or
>
> 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 no space to accomdate these new properties.
> This patch adds realloc functions in fdtput to allocate new space in fdt
> when it detects a shortage in space for new value or node. With this
> patch, fdtput can insert a new node or property or extend a property
> with new value greater than original size. Also it packs the final blob
> to clean up any extra padding.
>
> 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 at st.com>
> CC: David Gibson <dwg at au1.ibm.com>
> ---
> fdtput.c | 87 +++++++++++++++++++++++++++++++++++++++++++++++++++----------
> 1 files changed, 72 insertions(+), 15 deletions(-)
>
> diff --git a/fdtput.c b/fdtput.c
> index f2197f5..3e17977 100644
> --- a/fdtput.c
> +++ b/fdtput.c
> @@ -131,19 +131,72 @@ 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,
> +#define ALIGN(x) (((x) + (FDT_TAGSIZE) - 1) & ~((FDT_TAGSIZE) - 1))
> +
> +static char *_realloc_fdt(char *fdt, int delta)
> +{
> + int new_sz;
> + char *new_fdt;
> +
> + if (!delta)
> + return fdt;
> +
> + new_sz = fdt_totalsize(fdt) + delta;
> + new_fdt = malloc(new_sz);
Might as well use realloc() here. fdt_open_into() is (by design) safe
in both the overlapping and non-overlapping cases.
Also, use xrealloc()/xmalloc() instead of plain realloc()/malloc().
Your caller doesn't actually handle the failure case here when this
returns NULL, and there's not a whole lot you can do, so the abort()
is about the best you can do, and simplifies the code here.
> + if (!new_fdt) {
> + fprintf(stderr, "Unable to allocate memory to new fdt\n");
> + return fdt;
> + }
> + fdt_open_into(fdt, new_fdt, new_sz);
> + free(fdt);
> + return new_fdt;
> +}
> +
> +static char *realloc_node(char *fdt, const char *name)
> +{
> + int delta = 0;
> + int newlen = strlen(name);
> + if (newlen)
I don't see any point to this test. Adding a node named "" would be
an error in other ways, but it would still require 8 bytes of extra
space.
> + delta = sizeof(struct fdt_node_header) +
> + ALIGN(newlen + 1) + FDT_TAGSIZE;
struct fdt_node_header already includes the FDT_BEGIN_NODE tag, so
you've allocated space for the tag twice.
> +
> + return _realloc_fdt(fdt, delta);
> +}
> +
> +static char *realloc_property(char *fdt, int nodeoffset,
> + const char *name, int newlen)
> +{
> + int delta = 0;
> + int oldlen = 0;
> +
> + 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 += ALIGN(newlen) - ALIGN(oldlen);
> +
> + return _realloc_fdt(fdt, delta);
> +}
> +
> +static int store_key_value(char **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);
> + err = fdt_setprop(*blob, node, property, buf, len);
> + if (err == -FDT_ERR_NOSPACE) {
> + *blob = 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 +214,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(char **blob, const char *in_path)
> {
> const char *path = in_path;
> const char *sep;
> @@ -177,10 +230,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,
> + *blob = realloc_node(*blob, path);
> + node = fdt_add_subnode_namelen(*blob, offset, path,
> sep - path);
> }
> if (node < 0) {
> @@ -203,7 +257,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(char **blob, const char *node_name)
> {
> int node = 0;
> char *p;
> @@ -215,15 +269,17 @@ static int create_node(void *blob, const char *node_name)
> }
> *p = '\0';
>
> + *blob = realloc_node(*blob, p + 1);
> +
> 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);
> + node = fdt_add_subnode(*blob, node, p + 1);
> if (node < 0) {
> report_error(p + 1, -1, node);
> return -1;
> @@ -250,23 +306,25 @@ 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(&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(&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(&blob, *arg);
> else
> - ret = create_node(blob, *arg);
> + ret = create_node(&blob, *arg);
> }
> break;
> }
> - if (ret >= 0)
> + if (ret >= 0) {
> + fdt_pack(blob);
> ret = utilfdt_write(filename, blob);
> + }
>
> free(blob);
> return ret;
> @@ -317,7 +375,6 @@ int main(int argc, char *argv[])
> * - rename node
> * - pack fdt before writing
> * - set amount of free space when writing
> - * - expand fdt if value doesn't fit
> */
> switch (c) {
> case 'c':
--
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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.ozlabs.org/pipermail/devicetree-discuss/attachments/20130429/13a973a9/attachment-0001.sig>
More information about the devicetree-discuss
mailing list