[PATCH] fdtput: add delete node and property function

[PATCH] fdtput: add delete node and property function

[Wang Long]

add the delete node and property function for fdtput.

usage:
1) delete a node
 # fdtput test.dtb -d node /chosen/son
2) delete a property
 # fdtput test.dtb -d prop /chosen/ prop_name

Signed-off-by: Wang Long <long.wanglong-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
---
 fdtput.c           | 94 ++++++++++++++++++++++++++++++++++++++++++++++++++++--
 tests/run_tests.sh | 17 ++++++++++
 2 files changed, 108 insertions(+), 3 deletions(-)

diff --git a/fdtput.c b/fdtput.c
index 2a8d674..28e1f55 100644
--- a/fdtput.c
+++ b/fdtput.c
@@ -32,6 +32,8 @@
 enum oper_type {
 	OPER_WRITE_PROP,		/* Write a property in a node */
 	OPER_CREATE_NODE,		/* Create a new node */
+	OPER_DELETE_NODE,		/* Delete a node */
+	OPER_DELETE_PROP,		/* Delete a property in a node */
 };
 
 struct display_info {
@@ -270,6 +272,60 @@ static int create_node(char **blob, const char *node_name)
 	return 0;
 }
 
+/**
+ * Delete a property of a node in the fdt.
+ *
+ * @param blob		FDT blob to write into
+ * @param node_name	Name of node which the delete property belongs to
+ * @param prop_name	Name of property to delete
+ * @return 0 on success, or -1 on failure
+ */
+static int delete_prop(char **blob, const char *node_name, const char *prop_name)
+{
+	int node = 0;
+
+	node = fdt_path_offset(*blob, node_name);
+	if (node < 0) {
+		report_error(node_name, -1, node);
+		return -1;
+	}
+
+	node = fdt_delprop(*blob, node, prop_name);
+
+	if (node < 0) {
+		report_error(node_name, -1, node);
+		return -1;
+	}
+
+	return 0;
+}
+
+/**
+ * Delete a node in the fdt.
+ *
+ * @param blob		FDT blob to write into
+ * @param node_name	Name of node to delete
+ * @return the delete node offset if found, or -1 on failure
+ */
+static int delete_node(char **blob, const char *node_name)
+{
+	int node = 0;
+
+	node = fdt_path_offset(*blob, node_name);
+	if (node < 0) {
+		report_error(node_name, -1, node);
+		return -1;
+	}
+
+	node = fdt_del_node(*blob, node);
+	if (node < 0) {
+		report_error(node_name, -1, node);
+		return -1;
+	}
+
+	return 0;
+}
+
 static int do_fdtput(struct display_info *disp, const char *filename,
 		    char **arg, int arg_count)
 {
@@ -302,6 +358,13 @@ static int do_fdtput(struct display_info *disp, const char *filename,
 				ret = create_node(&blob, *arg);
 		}
 		break;
+	case OPER_DELETE_NODE:
+		for (; ret >= 0 && arg_count--; arg++)
+			ret = delete_node(&blob, *arg);
+		break;
+	case OPER_DELETE_PROP:
+		ret = delete_prop(&blob, *arg, arg[1]);
+		break;
 	}
 	if (ret >= 0) {
 		fdt_pack(blob);
@@ -312,17 +375,29 @@ static int do_fdtput(struct display_info *disp, const char *filename,
 	return ret;
 }
 
+/*
+ * This is a usage message fragment for the -d option.
+ *
+ */
+#define USAGE_DELETE_MSG \
+        "<delete>  node, prop\n" \
+	"\t  node: delete node\n" \
+	"\t  prop: delete property\n"
+
 /* Usage related data. */
 static const char usage_synopsis[] =
 	"write a property value to a device tree\n"
 	"	fdtput <options> <dt file> <node> <property> [<value>...]\n"
 	"	fdtput -c <options> <dt file> [<node>...]\n"
+	"	fdtput -d <options> <dt file> <node> <property>\n"
 	"\n"
 	"The command line arguments are joined together into a single value.\n"
-	USAGE_TYPE_MSG;
-static const char usage_short_opts[] = "cpt:v" USAGE_COMMON_SHORT_OPTS;
+	USAGE_DELETE_MSG
+	USAGE_TYPE_MSG
+static const char usage_short_opts[] = "cd:pt:v" USAGE_COMMON_SHORT_OPTS;
 static struct option const usage_long_opts[] = {
 	{"create",           no_argument, NULL, 'c'},
+	{"delete",            a_argument, NULL, 'd'},
 	{"auto-path",        no_argument, NULL, 'p'},
 	{"type",              a_argument, NULL, 't'},
 	{"verbose",          no_argument, NULL, 'v'},
@@ -330,6 +405,7 @@ static struct option const usage_long_opts[] = {
 };
 static const char * const usage_opts_help[] = {
 	"Create nodes if they don't already exist",
+	"Delete nodes or property if they already exist",
 	"Automatically create nodes as needed for the node path",
 	"Type of data",
 	"Display each value decoded from command line",
@@ -348,7 +424,6 @@ int main(int argc, char *argv[])
 	while ((opt = util_getopt_long()) != EOF) {
 		/*
 		 * TODO: add options to:
-		 * - delete property
 		 * - delete node (optionally recursively)
 		 * - rename node
 		 * - pack fdt before writing
@@ -360,6 +435,14 @@ int main(int argc, char *argv[])
 		case 'c':
 			disp.oper = OPER_CREATE_NODE;
 			break;
+		case 'd':
+			if (strcmp(optarg, "node") == 0)
+				disp.oper = OPER_DELETE_NODE;
+			else if (strcmp(optarg, "prop") == 0)
+				disp.oper = OPER_DELETE_PROP;
+			else
+				usage("Invalid delete string");
+			break;
 		case 'p':
 			disp.auto_path = 1;
 			break;
@@ -390,6 +473,11 @@ int main(int argc, char *argv[])
 			usage("missing property");
 	}
 
+	if (disp.oper == OPER_DELETE_NODE || disp.oper == OPER_DELETE_PROP) {
+		if (argc < 1)
+			usage("missing node or property");
+	}
+
 	if (do_fdtput(&disp, filename, argv, argc))
 		return 1;
 	return 0;
diff --git a/tests/run_tests.sh b/tests/run_tests.sh
index ace6e4f..f633e62 100755
--- a/tests/run_tests.sh
+++ b/tests/run_tests.sh
@@ -610,6 +610,23 @@ fdtput_tests () {
     run_wrap_test $DTPUT $dtb -cp /chosen
     run_wrap_test $DTPUT $dtb -cp /chosen/son
 
+    # Start again with a fresh dtb
+    run_dtc_test -O dtb -p $(stat -c %s $text) -o $dtb $dts
+
+    # Node delete
+    run_wrap_test $DTPUT $dtb -c /chosen/son /chosen/daughter
+    run_wrap_test $DTPUT $dtb -d node  /chosen/son /chosen/daughter
+
+    # Delete the not exist node
+    run_wrap_error_test $DTPUT $dtb -d node  /not/exist/node
+
+    # Property delete
+    run_fdtput_test "eva" $dtb /chosen/ name "" -ts "eva"
+    run_wrap_test $DTPUT $dtb -d prop /chosen/ name
+
+    # Delete the not exist property
+    run_wrap_error_test $DTPUT $dtb -d prop  /chosen   notexistprop
+
     # TODO: Add tests for verbose mode?
 }
 
-- 
1.8.3.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree-compiler" 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] fdtput: add delete node and property function

[Simon Glass]

Hi Wang,

On 13 January 2015 at 19:53, Wang Long <long.wanglong-hv44wF8Li93QT0dZR+AlfA@public.gmane.org> wrote:
> add the delete node and property function for fdtput.
>
> usage:
> 1) delete a node
>  # fdtput test.dtb -d node /chosen/son
> 2) delete a property
>  # fdtput test.dtb -d prop /chosen/ prop_name

This is a great addition!

I know David is keen on single character flags. But would it be better
to have a separate delete option, e.g.

-r to remove a node
-d to delete a property

>
> Signed-off-by: Wang Long <long.wanglong-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
> ---
>  fdtput.c           | 94 ++++++++++++++++++++++++++++++++++++++++++++++++++++--
>  tests/run_tests.sh | 17 ++++++++++
>  2 files changed, 108 insertions(+), 3 deletions(-)
>
> diff --git a/fdtput.c b/fdtput.c
> index 2a8d674..28e1f55 100644
> --- a/fdtput.c
> +++ b/fdtput.c
> @@ -32,6 +32,8 @@
>  enum oper_type {
>         OPER_WRITE_PROP,                /* Write a property in a node */
>         OPER_CREATE_NODE,               /* Create a new node */
> +       OPER_DELETE_NODE,               /* Delete a node */
> +       OPER_DELETE_PROP,               /* Delete a property in a node */
>  };
>
>  struct display_info {
> @@ -270,6 +272,60 @@ static int create_node(char **blob, const char *node_name)
>         return 0;
>  }
>
> +/**
> + * Delete a property of a node in the fdt.
> + *
> + * @param blob         FDT blob to write into
> + * @param node_name    Name of node which the delete property belongs to
> + * @param prop_name    Name of property to delete
> + * @return 0 on success, or -1 on failure
> + */
> +static int delete_prop(char **blob, const char *node_name, const char *prop_name)

Can you just pass char *blob? Why the double pointer? Same with the
next function.

> +{
> +       int node = 0;
> +
> +       node = fdt_path_offset(*blob, node_name);
> +       if (node < 0) {
> +               report_error(node_name, -1, node);
> +               return -1;
> +       }
> +
> +       node = fdt_delprop(*blob, node, prop_name);
> +
> +       if (node < 0) {
> +               report_error(node_name, -1, node);
> +               return -1;
> +       }
> +
> +       return 0;
> +}
> +
> +/**
> + * Delete a node in the fdt.
> + *
> + * @param blob         FDT blob to write into
> + * @param node_name    Name of node to delete
> + * @return the delete node offset if found, or -1 on failure
> + */
> +static int delete_node(char **blob, const char *node_name)
> +{
> +       int node = 0;
> +
> +       node = fdt_path_offset(*blob, node_name);
> +       if (node < 0) {
> +               report_error(node_name, -1, node);
> +               return -1;
> +       }
> +
> +       node = fdt_del_node(*blob, node);
> +       if (node < 0) {
> +               report_error(node_name, -1, node);
> +               return -1;
> +       }
> +
> +       return 0;
> +}
> +
>  static int do_fdtput(struct display_info *disp, const char *filename,
>                     char **arg, int arg_count)
>  {
> @@ -302,6 +358,13 @@ static int do_fdtput(struct display_info *disp, const char *filename,
>                                 ret = create_node(&blob, *arg);
>                 }
>                 break;
> +       case OPER_DELETE_NODE:
> +               for (; ret >= 0 && arg_count--; arg++)
> +                       ret = delete_node(&blob, *arg);
> +               break;
> +       case OPER_DELETE_PROP:
> +               ret = delete_prop(&blob, *arg, arg[1]);
> +               break;
>         }
>         if (ret >= 0) {
>                 fdt_pack(blob);
> @@ -312,17 +375,29 @@ static int do_fdtput(struct display_info *disp, const char *filename,
>         return ret;
>  }
>
> +/*
> + * This is a usage message fragment for the -d option.
> + *
> + */
> +#define USAGE_DELETE_MSG \
> +        "<delete>  node, prop\n" \
> +       "\t  node: delete node\n" \
> +       "\t  prop: delete property\n"
> +
>  /* Usage related data. */
>  static const char usage_synopsis[] =
>         "write a property value to a device tree\n"
>         "       fdtput <options> <dt file> <node> <property> [<value>...]\n"
>         "       fdtput -c <options> <dt file> [<node>...]\n"
> +       "       fdtput -d <options> <dt file> <node> <property>\n"
>         "\n"
>         "The command line arguments are joined together into a single value.\n"
> -       USAGE_TYPE_MSG;
> -static const char usage_short_opts[] = "cpt:v" USAGE_COMMON_SHORT_OPTS;
> +       USAGE_DELETE_MSG
> +       USAGE_TYPE_MSG
> +static const char usage_short_opts[] = "cd:pt:v" USAGE_COMMON_SHORT_OPTS;
>  static struct option const usage_long_opts[] = {
>         {"create",           no_argument, NULL, 'c'},
> +       {"delete",            a_argument, NULL, 'd'},
>         {"auto-path",        no_argument, NULL, 'p'},
>         {"type",              a_argument, NULL, 't'},
>         {"verbose",          no_argument, NULL, 'v'},
> @@ -330,6 +405,7 @@ static struct option const usage_long_opts[] = {
>  };
>  static const char * const usage_opts_help[] = {
>         "Create nodes if they don't already exist",
> +       "Delete nodes or property if they already exist",
>         "Automatically create nodes as needed for the node path",
>         "Type of data",
>         "Display each value decoded from command line",
> @@ -348,7 +424,6 @@ int main(int argc, char *argv[])
>         while ((opt = util_getopt_long()) != EOF) {
>                 /*
>                  * TODO: add options to:
> -                * - delete property
>                  * - delete node (optionally recursively)
>                  * - rename node
>                  * - pack fdt before writing
> @@ -360,6 +435,14 @@ int main(int argc, char *argv[])
>                 case 'c':
>                         disp.oper = OPER_CREATE_NODE;
>                         break;
> +               case 'd':
> +                       if (strcmp(optarg, "node") == 0)
> +                               disp.oper = OPER_DELETE_NODE;
> +                       else if (strcmp(optarg, "prop") == 0)
> +                               disp.oper = OPER_DELETE_PROP;
> +                       else
> +                               usage("Invalid delete string");
> +                       break;
>                 case 'p':
>                         disp.auto_path = 1;
>                         break;
> @@ -390,6 +473,11 @@ int main(int argc, char *argv[])
>                         usage("missing property");
>         }
>
> +       if (disp.oper == OPER_DELETE_NODE || disp.oper == OPER_DELETE_PROP) {
> +               if (argc < 1)
> +                       usage("missing node or property");
> +       }
> +
>         if (do_fdtput(&disp, filename, argv, argc))
>                 return 1;
>         return 0;
> diff --git a/tests/run_tests.sh b/tests/run_tests.sh
> index ace6e4f..f633e62 100755
> --- a/tests/run_tests.sh
> +++ b/tests/run_tests.sh
> @@ -610,6 +610,23 @@ fdtput_tests () {
>      run_wrap_test $DTPUT $dtb -cp /chosen
>      run_wrap_test $DTPUT $dtb -cp /chosen/son
>
> +    # Start again with a fresh dtb
> +    run_dtc_test -O dtb -p $(stat -c %s $text) -o $dtb $dts

Do you need to do this?

> +
> +    # Node delete
> +    run_wrap_test $DTPUT $dtb -c /chosen/son /chosen/daughter

These tests seems sufficient, but a few ideas:

Perhaps here you could list the node properties to make sure the node
has been created?

> +    run_wrap_test $DTPUT $dtb -d node  /chosen/son /chosen/daughter
> +
> +    # Delete the not exist node
> +    run_wrap_error_test $DTPUT $dtb -d node  /not/exist/node
> +
> +    # Property delete
> +    run_fdtput_test "eva" $dtb /chosen/ name "" -ts "eva"
> +    run_wrap_test $DTPUT $dtb -d prop /chosen/ name

Here you could list the properties to make suere there is none?

> +
> +    # Delete the not exist property

nit: s/not exist/non-existent/

> +    run_wrap_error_test $DTPUT $dtb -d prop  /chosen   notexistprop
> +
>      # TODO: Add tests for verbose mode?
>  }
>
> --

Regards,
Simon
--
To unsubscribe from this list: send the line "unsubscribe devicetree-compiler" 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] fdtput: add delete node and property function

[long.wanglong]

On 2015/1/14 12:07, Simon Glass wrote:
> Hi Wang,
> 
> On 13 January 2015 at 19:53, Wang Long <long.wanglong-hv44wF8Li93QT0dZR+AlfA@public.gmane.org> wrote:
>> add the delete node and property function for fdtput.
>>
>> usage:
>> 1) delete a node
>>  # fdtput test.dtb -d node /chosen/son
>> 2) delete a property
>>  # fdtput test.dtb -d prop /chosen/ prop_name
> 
> This is a great addition!
> 
> I know David is keen on single character flags. But would it be better
> to have a separate delete option, e.g.
> 
> -r to remove a node
> -d to delete a property
> 
ok

i will use single character flags in next version of the patch.

>>
>> Signed-off-by: Wang Long <long.wanglong-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
>> ---
>>  fdtput.c           | 94 ++++++++++++++++++++++++++++++++++++++++++++++++++++--
>>  tests/run_tests.sh | 17 ++++++++++
>>  2 files changed, 108 insertions(+), 3 deletions(-)
>>
>> diff --git a/fdtput.c b/fdtput.c
>> index 2a8d674..28e1f55 100644
>> --- a/fdtput.c
>> +++ b/fdtput.c
>> @@ -32,6 +32,8 @@
>>  enum oper_type {
>>         OPER_WRITE_PROP,                /* Write a property in a node */
>>         OPER_CREATE_NODE,               /* Create a new node */
>> +       OPER_DELETE_NODE,               /* Delete a node */
>> +       OPER_DELETE_PROP,               /* Delete a property in a node */
>>  };
>>
>>  struct display_info {
>> @@ -270,6 +272,60 @@ static int create_node(char **blob, const char *node_name)
>>         return 0;
>>  }
>>
>> +/**
>> + * Delete a property of a node in the fdt.
>> + *
>> + * @param blob         FDT blob to write into
>> + * @param node_name    Name of node which the delete property belongs to
>> + * @param prop_name    Name of property to delete
>> + * @return 0 on success, or -1 on failure
>> + */
>> +static int delete_prop(char **blob, const char *node_name, const char *prop_name)
> 
> Can you just pass char *blob? Why the double pointer? Same with the
> next function.
> 
hi,Simon

the function create_paths and create_node use the double pointer.

 static int create_paths(char **blob, const char *in_path)
 static int create_node(char **blob, const char *node_name)

In order to maintain a consistent coding style, i use char **blob instead of char *blob.
should we update char **blob  to char *blob in all functions?


>> +{
>> +       int node = 0;
>> +
>> +       node = fdt_path_offset(*blob, node_name);
>> +       if (node < 0) {
>> +               report_error(node_name, -1, node);
>> +               return -1;
>> +       }
>> +
>> +       node = fdt_delprop(*blob, node, prop_name);
>> +
>> +       if (node < 0) {
>> +               report_error(node_name, -1, node);
>> +               return -1;
>> +       }
>> +
>> +       return 0;
>> +}
>> +
>> +/**
>> + * Delete a node in the fdt.
>> + *
>> + * @param blob         FDT blob to write into
>> + * @param node_name    Name of node to delete
>> + * @return the delete node offset if found, or -1 on failure
>> + */
>> +static int delete_node(char **blob, const char *node_name)
>> +{
>> +       int node = 0;
>> +
>> +       node = fdt_path_offset(*blob, node_name);
>> +       if (node < 0) {
>> +               report_error(node_name, -1, node);
>> +               return -1;
>> +       }
>> +
>> +       node = fdt_del_node(*blob, node);
>> +       if (node < 0) {
>> +               report_error(node_name, -1, node);
>> +               return -1;
>> +       }
>> +
>> +       return 0;
>> +}
>> +
>>  static int do_fdtput(struct display_info *disp, const char *filename,
>>                     char **arg, int arg_count)
>>  {
>> @@ -302,6 +358,13 @@ static int do_fdtput(struct display_info *disp, const char *filename,
>>                                 ret = create_node(&blob, *arg);
>>                 }
>>                 break;
>> +       case OPER_DELETE_NODE:
>> +               for (; ret >= 0 && arg_count--; arg++)
>> +                       ret = delete_node(&blob, *arg);
>> +               break;
>> +       case OPER_DELETE_PROP:
>> +               ret = delete_prop(&blob, *arg, arg[1]);
>> +               break;
>>         }
>>         if (ret >= 0) {
>>                 fdt_pack(blob);
>> @@ -312,17 +375,29 @@ static int do_fdtput(struct display_info *disp, const char *filename,
>>         return ret;
>>  }
>>
>> +/*
>> + * This is a usage message fragment for the -d option.
>> + *
>> + */
>> +#define USAGE_DELETE_MSG \
>> +        "<delete>  node, prop\n" \
>> +       "\t  node: delete node\n" \
>> +       "\t  prop: delete property\n"
>> +
>>  /* Usage related data. */
>>  static const char usage_synopsis[] =
>>         "write a property value to a device tree\n"
>>         "       fdtput <options> <dt file> <node> <property> [<value>...]\n"
>>         "       fdtput -c <options> <dt file> [<node>...]\n"
>> +       "       fdtput -d <options> <dt file> <node> <property>\n"
>>         "\n"
>>         "The command line arguments are joined together into a single value.\n"
>> -       USAGE_TYPE_MSG;
>> -static const char usage_short_opts[] = "cpt:v" USAGE_COMMON_SHORT_OPTS;
>> +       USAGE_DELETE_MSG
>> +       USAGE_TYPE_MSG
>> +static const char usage_short_opts[] = "cd:pt:v" USAGE_COMMON_SHORT_OPTS;
>>  static struct option const usage_long_opts[] = {
>>         {"create",           no_argument, NULL, 'c'},
>> +       {"delete",            a_argument, NULL, 'd'},
>>         {"auto-path",        no_argument, NULL, 'p'},
>>         {"type",              a_argument, NULL, 't'},
>>         {"verbose",          no_argument, NULL, 'v'},
>> @@ -330,6 +405,7 @@ static struct option const usage_long_opts[] = {
>>  };
>>  static const char * const usage_opts_help[] = {
>>         "Create nodes if they don't already exist",
>> +       "Delete nodes or property if they already exist",
>>         "Automatically create nodes as needed for the node path",
>>         "Type of data",
>>         "Display each value decoded from command line",
>> @@ -348,7 +424,6 @@ int main(int argc, char *argv[])
>>         while ((opt = util_getopt_long()) != EOF) {
>>                 /*
>>                  * TODO: add options to:
>> -                * - delete property
>>                  * - delete node (optionally recursively)
>>                  * - rename node
>>                  * - pack fdt before writing
>> @@ -360,6 +435,14 @@ int main(int argc, char *argv[])
>>                 case 'c':
>>                         disp.oper = OPER_CREATE_NODE;
>>                         break;
>> +               case 'd':
>> +                       if (strcmp(optarg, "node") == 0)
>> +                               disp.oper = OPER_DELETE_NODE;
>> +                       else if (strcmp(optarg, "prop") == 0)
>> +                               disp.oper = OPER_DELETE_PROP;
>> +                       else
>> +                               usage("Invalid delete string");
>> +                       break;
>>                 case 'p':
>>                         disp.auto_path = 1;
>>                         break;
>> @@ -390,6 +473,11 @@ int main(int argc, char *argv[])
>>                         usage("missing property");
>>         }
>>
>> +       if (disp.oper == OPER_DELETE_NODE || disp.oper == OPER_DELETE_PROP) {
>> +               if (argc < 1)
>> +                       usage("missing node or property");
>> +       }
>> +
>>         if (do_fdtput(&disp, filename, argv, argc))
>>                 return 1;
>>         return 0;
>> diff --git a/tests/run_tests.sh b/tests/run_tests.sh
>> index ace6e4f..f633e62 100755
>> --- a/tests/run_tests.sh
>> +++ b/tests/run_tests.sh
>> @@ -610,6 +610,23 @@ fdtput_tests () {
>>      run_wrap_test $DTPUT $dtb -cp /chosen
>>      run_wrap_test $DTPUT $dtb -cp /chosen/son
>>
>> +    # Start again with a fresh dtb
>> +    run_dtc_test -O dtb -p $(stat -c %s $text) -o $dtb $dts
> 
> Do you need to do this?

ok, to test the function of deleting node and property, using an non-fresh dtb
is also ok.

> 
>> +
>> +    # Node delete
>> +    run_wrap_test $DTPUT $dtb -c /chosen/son /chosen/daughter
> 
> These tests seems sufficient, but a few ideas:
> 
> Perhaps here you could list the node properties to make sure the node
> has been created?

ok

> 
>> +    run_wrap_test $DTPUT $dtb -d node  /chosen/son /chosen/daughter
>> +
>> +    # Delete the not exist node
>> +    run_wrap_error_test $DTPUT $dtb -d node  /not/exist/node
>> +
>> +    # Property delete
>> +    run_fdtput_test "eva" $dtb /chosen/ name "" -ts "eva"
>> +    run_wrap_test $DTPUT $dtb -d prop /chosen/ name
> 
> Here you could list the properties to make suere there is none?
> 
ok

>> +
>> +    # Delete the not exist property
> 
> nit: s/not exist/non-existent/
ok thanks
> 
>> +    run_wrap_error_test $DTPUT $dtb -d prop  /chosen   notexistprop
>> +
>>      # TODO: Add tests for verbose mode?
>>  }
>>
>> --
> 
> Regards,
> Simon
> 
> .
> 

Best Regards
Wang Long


--
To unsubscribe from this list: send the line "unsubscribe devicetree-compiler" 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] fdtput: add delete node and property function

[Simon Glass]

Hi Wang,

On 13 January 2015 at 21:25, long.wanglong <long.wanglong-hv44wF8Li93QT0dZR+AlfA@public.gmane.org> wrote:
> On 2015/1/14 12:07, Simon Glass wrote:
>> Hi Wang,
>>
>> On 13 January 2015 at 19:53, Wang Long <long.wanglong-hv44wF8Li93QT0dZR+AlfA@public.gmane.org> wrote:
>>> add the delete node and property function for fdtput.
>>>
>>> usage:
>>> 1) delete a node
>>>  # fdtput test.dtb -d node /chosen/son
>>> 2) delete a property
>>>  # fdtput test.dtb -d prop /chosen/ prop_name
>>
>> This is a great addition!
>>
>> I know David is keen on single character flags. But would it be better
>> to have a separate delete option, e.g.
>>
>> -r to remove a node
>> -d to delete a property
>>
> ok
>
> i will use single character flags in next version of the patch.

You might want to wait to see if David has some comments before
sending the next version.

>
>>>
>>> Signed-off-by: Wang Long <long.wanglong-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
>>> ---
>>>  fdtput.c           | 94 ++++++++++++++++++++++++++++++++++++++++++++++++++++--
>>>  tests/run_tests.sh | 17 ++++++++++
>>>  2 files changed, 108 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/fdtput.c b/fdtput.c
>>> index 2a8d674..28e1f55 100644
>>> --- a/fdtput.c
>>> +++ b/fdtput.c
>>> @@ -32,6 +32,8 @@
>>>  enum oper_type {
>>>         OPER_WRITE_PROP,                /* Write a property in a node */
>>>         OPER_CREATE_NODE,               /* Create a new node */
>>> +       OPER_DELETE_NODE,               /* Delete a node */
>>> +       OPER_DELETE_PROP,               /* Delete a property in a node */
>>>  };
>>>
>>>  struct display_info {
>>> @@ -270,6 +272,60 @@ static int create_node(char **blob, const char *node_name)
>>>         return 0;
>>>  }
>>>
>>> +/**
>>> + * Delete a property of a node in the fdt.
>>> + *
>>> + * @param blob         FDT blob to write into
>>> + * @param node_name    Name of node which the delete property belongs to
>>> + * @param prop_name    Name of property to delete
>>> + * @return 0 on success, or -1 on failure
>>> + */
>>> +static int delete_prop(char **blob, const char *node_name, const char *prop_name)
>>
>> Can you just pass char *blob? Why the double pointer? Same with the
>> next function.
>>
> hi,Simon
>
> the function create_paths and create_node use the double pointer.
>
>  static int create_paths(char **blob, const char *in_path)
>  static int create_node(char **blob, const char *node_name)
>
> In order to maintain a consistent coding style, i use char **blob instead of char *blob.
> should we update char **blob  to char *blob in all functions?

When expanding the tree (e.g. by adding nodes or properties) we might
need to relocate it. But since you are deleting things, that isn't
necessary.

Regards
Simon
--
To unsubscribe from this list: send the line "unsubscribe devicetree-compiler" 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] fdtput: add delete node and property function

[David Gibson]

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

On Tue, Jan 13, 2015 at 08:07:59PM -0800, Simon Glass wrote:
> Hi Wang,
> 
> On 13 January 2015 at 19:53, Wang Long <long.wanglong-hv44wF8Li93QT0dZR+AlfA@public.gmane.org> wrote:
> > add the delete node and property function for fdtput.
> >
> > usage:
> > 1) delete a node
> >  # fdtput test.dtb -d node /chosen/son
> > 2) delete a property
> >  # fdtput test.dtb -d prop /chosen/ prop_name
> 
> This is a great addition!
> 
> I know David is keen on single character flags. But would it be better
> to have a separate delete option, e.g.

Well.. it's not that I'm actually keen on them.  Just that I haven't
thought the bother of another dependency adding a long options library
was worth it so far.

> -r to remove a node
> -d to delete a property

I like that suggestion better than "-d node" and "-d prop".

But I wonder if it might be even simpler to just base it on the number
of arguments so:

$ fdtput foo.dtb -d /node@1234 some-property

will delete a single property, but

$ fdtput foo.dtb -d /node@1234

will delete the whole node.

-- 
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: Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] fdtput: add delete node and property function

[long.wanglong]

On 2015/1/15 11:08, David Gibson wrote:
> On Tue, Jan 13, 2015 at 08:07:59PM -0800, Simon Glass wrote:
>> Hi Wang,
>>
>> On 13 January 2015 at 19:53, Wang Long <long.wanglong-hv44wF8Li93QT0dZR+AlfA@public.gmane.org> wrote:
>>> add the delete node and property function for fdtput.
>>>
>>> usage:
>>> 1) delete a node
>>>  # fdtput test.dtb -d node /chosen/son
>>> 2) delete a property
>>>  # fdtput test.dtb -d prop /chosen/ prop_name
>>
>> This is a great addition!
>>
>> I know David is keen on single character flags. But would it be better
>> to have a separate delete option, e.g.
> 
> Well.. it's not that I'm actually keen on them.  Just that I haven't
> thought the bother of another dependency adding a long options library
> was worth it so far.
> 
>> -r to remove a node
>> -d to delete a property
> 
> I like that suggestion better than "-d node" and "-d prop".
> 

ok.

> But I wonder if it might be even simpler to just base it on the number
> of arguments so:
> 
> $ fdtput foo.dtb -d /node@1234 some-property
> 
> will delete a single property, but
> 
> $ fdtput foo.dtb -d /node@1234
> 
> will delete the whole node.
> 

Hi David Gibson,

if we decide to delete node or property based on the number of the arguments.
we just delete a node or a property at once.

how about the following suggestion:

$ fdtput -r <options> <dt file> [<node>...]

will delete several nodes,

$ fdtput -d <options> <dt file> <node> [<property>...]

will delete several properties in a node.

the `fdtput` can create several nodes at once. so i think that it should to delete
several nodes at once.


Best Regards
Wang Long






--
To unsubscribe from this list: send the line "unsubscribe devicetree-compiler" 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] fdtput: add delete node and property function

[Simon Glass]

Hi,

On 14 January 2015 at 21:11, long.wanglong <long.wanglong-hv44wF8Li93QT0dZR+AlfA@public.gmane.org> wrote:
> On 2015/1/15 11:08, David Gibson wrote:
>> On Tue, Jan 13, 2015 at 08:07:59PM -0800, Simon Glass wrote:
>>> Hi Wang,
>>>
>>> On 13 January 2015 at 19:53, Wang Long <long.wanglong-hv44wF8Li93QT0dZR+AlfA@public.gmane.org> wrote:
>>>> add the delete node and property function for fdtput.
>>>>
>>>> usage:
>>>> 1) delete a node
>>>>  # fdtput test.dtb -d node /chosen/son
>>>> 2) delete a property
>>>>  # fdtput test.dtb -d prop /chosen/ prop_name
>>>
>>> This is a great addition!
>>>
>>> I know David is keen on single character flags. But would it be better
>>> to have a separate delete option, e.g.
>>
>> Well.. it's not that I'm actually keen on them.  Just that I haven't
>> thought the bother of another dependency adding a long options library
>> was worth it so far.
>>
>>> -r to remove a node
>>> -d to delete a property
>>
>> I like that suggestion better than "-d node" and "-d prop".
>>
>
> ok.
>
>> But I wonder if it might be even simpler to just base it on the number
>> of arguments so:
>>
>> $ fdtput foo.dtb -d /node@1234 some-property
>>
>> will delete a single property, but
>>
>> $ fdtput foo.dtb -d /node@1234
>>
>> will delete the whole node.
>>
>
> Hi David Gibson,
>
> if we decide to delete node or property based on the number of the arguments.
> we just delete a node or a property at once.
>
> how about the following suggestion:
>
> $ fdtput -r <options> <dt file> [<node>...]
>
> will delete several nodes,
>
> $ fdtput -d <options> <dt file> <node> [<property>...]
>
> will delete several properties in a node.
>
> the `fdtput` can create several nodes at once. so i think that it should to delete
> several nodes at once.

Well, I like that idea.

Regards,
Simon
--
To unsubscribe from this list: send the line "unsubscribe devicetree-compiler" 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] fdtput: add delete node and property function

[David Gibson]

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

On Thu, Jan 15, 2015 at 12:11:18PM +0800, long.wanglong wrote:
> On 2015/1/15 11:08, David Gibson wrote:
> > On Tue, Jan 13, 2015 at 08:07:59PM -0800, Simon Glass wrote:
> >> Hi Wang,
> >>
> >> On 13 January 2015 at 19:53, Wang Long <long.wanglong-hv44wF8Li93QT0dZR+AlfA@public.gmane.org> wrote:
> >>> add the delete node and property function for fdtput.
> >>>
> >>> usage:
> >>> 1) delete a node
> >>>  # fdtput test.dtb -d node /chosen/son
> >>> 2) delete a property
> >>>  # fdtput test.dtb -d prop /chosen/ prop_name
> >>
> >> This is a great addition!
> >>
> >> I know David is keen on single character flags. But would it be better
> >> to have a separate delete option, e.g.
> > 
> > Well.. it's not that I'm actually keen on them.  Just that I haven't
> > thought the bother of another dependency adding a long options library
> > was worth it so far.
> > 
> >> -r to remove a node
> >> -d to delete a property
> > 
> > I like that suggestion better than "-d node" and "-d prop".
> > 
> 
> ok.
> 
> > But I wonder if it might be even simpler to just base it on the number
> > of arguments so:
> > 
> > $ fdtput foo.dtb -d /node@1234 some-property
> > 
> > will delete a single property, but
> > 
> > $ fdtput foo.dtb -d /node@1234
> > 
> > will delete the whole node.
> > 
> 
> Hi David Gibson,
> 
> if we decide to delete node or property based on the number of the arguments.
> we just delete a node or a property at once.
> 
> how about the following suggestion:
> 
> $ fdtput -r <options> <dt file> [<node>...]
> 
> will delete several nodes,
> 
> $ fdtput -d <options> <dt file> <node> [<property>...]
> 
> will delete several properties in a node.
> 
> the `fdtput` can create several nodes at once. so i think that it should to delete
> several nodes at once.

Good point, I forgot that the other options can do multiple operations
in a single command.

Your suggestion sounds good to me.

-- 
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: Type: application/pgp-signature, Size: 819 bytes --]