[PATCH v3 0/6] Fixes for nv command handling

[PATCH v3 0/6] Fixes for nv command handling

[Enrico Jorns]

This series adds a couple of fixes and enhancement in option parsing and error
handling for the 'nv' command.

CHANGES:

* Add missing curly braces in 1/5
* properly introduce and use NV abbreviation
* re-align help text

Enrico Jorns (6):
  commands: nv: fail with verbose message if invoked without args
  commands: nv: call nvvar_print() only if no argument is given
  commands: nv: assure error code will be returned when an error
    occurred
  common: globvar: let nvvar_remove() report non-existing variable
  commands: nv: adjust command help
  command: nv: rewrap help text lines

 commands/nv.c      | 45 +++++++++++++++++++++++++++++----------------
 common/globalvar.c |  4 +++-
 2 files changed, 32 insertions(+), 17 deletions(-)

-- 
2.11.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 1/6] commands: nv: fail with verbose message if invoked without args

[Enrico Jorns]

Signed-off-by: Enrico Jorns <ejo@pengutronix.de>
---
 commands/nv.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/commands/nv.c b/commands/nv.c
index 37cdb96647..798b1eaa04 100644
--- a/commands/nv.c
+++ b/commands/nv.c
@@ -55,8 +55,10 @@ static int do_nv(int argc, char *argv[])
 	argc -= optind;
 	argv += optind;
 
-	if (argc < 1)
+	if (argc < 1) {
+		printf("Invalid usage: Missing argument");
 		return COMMAND_ERROR_USAGE;
+	}
 
 	for (i = 0; i < argc; i++) {
 		value = strchr(argv[0], '=');
-- 
2.11.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 2/6] commands: nv: call nvvar_print() only if no argument is given

[Enrico Jorns]

Printing the nv variables (before performing any modification!) is useless
and irritating.

Signed-off-by: Enrico Jorns <ejo@pengutronix.de>
---
 commands/nv.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/commands/nv.c b/commands/nv.c
index 798b1eaa04..2e6d079357 100644
--- a/commands/nv.c
+++ b/commands/nv.c
@@ -44,14 +44,15 @@ static int do_nv(int argc, char *argv[])
 		}
 	}
 
-	if (do_save)
-		return nvvar_save();
-
-	if (argc == optind) {
+	if (argc == 1) {
 		nvvar_print();
 		return 0;
 	}
 
+	if (do_save) {
+		return nvvar_save();
+	}
+
 	argc -= optind;
 	argv += optind;
 
-- 
2.11.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 3/6] commands: nv: assure error code will be returned when an error occurred

[Enrico Jorns]

Due to iteration over possibly multiple variables, the return value of
an individual call to nvvar_remove() / nvvar_add() gets lost.

This will result in do_nv() returning success (0) if any variable
processing failed as long as processing of the last variable succeeded.

Processing will not be aborted on individual errors, such as the 'cp'
handles it for invalid files.

Signed-off-by: Enrico Jorns <ejo@pengutronix.de>
---
 commands/nv.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/commands/nv.c b/commands/nv.c
index 2e6d079357..ebef97eae7 100644
--- a/commands/nv.c
+++ b/commands/nv.c
@@ -28,7 +28,7 @@ static int do_nv(int argc, char *argv[])
 {
 	int opt;
 	int do_remove = 0, do_save = 0;
-	int ret, i;
+	int failed = 0, i;
 	char *value;
 
 	while ((opt = getopt(argc, argv, "rs")) > 0) {
@@ -62,19 +62,29 @@ static int do_nv(int argc, char *argv[])
 	}
 
 	for (i = 0; i < argc; i++) {
+		int ret;
 		value = strchr(argv[0], '=');
 		if (value) {
 			*value = 0;
 			value++;
 		}
 
-		if (do_remove)
+		if (do_remove) {
 			ret = nvvar_remove(argv[i]);
-		else
+			if (ret) {
+				printf("Failed removing %s: %s\n", argv[i], strerror(-ret));
+				failed = 1;
+			}
+		} else {
 			ret = nvvar_add(argv[i], value);
+			if (ret) {
+				printf("Failed adding %s: %s\n", argv[i], strerror(-ret));
+				failed = 1;
+			}
+		}
 	}
 
-	return ret;
+	return failed;
 }
 
 BAREBOX_CMD_HELP_START(nv)
-- 
2.11.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 4/6] common: globvar: let nvvar_remove() report non-existing variable

[Enrico Jorns]

The former implementation did not allow to detect whether the call to
nvvar_remove() succeeded or failed and always returned 0.

This changes the implementation to return 0 only if a variable with the
given name was found and return ENOENT otherwise.

Signed-off-by: Enrico Jorns <ejo@pengutronix.de>
---
 common/globalvar.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/common/globalvar.c b/common/globalvar.c
index fdfaf76fae..ee756e5140 100644
--- a/common/globalvar.c
+++ b/common/globalvar.c
@@ -261,9 +261,11 @@ int nvvar_remove(const char *name)
 
 		unlink(fname);
 		free(fname);
+
+		return 0;
 	}
 
-	return 0;
+	return -ENOENT;
 }
 
 int nvvar_load(void)
-- 
2.11.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 5/6] commands: nv: adjust command help

[Enrico Jorns]

Help did not point out that -r option requires n arguments and did not
mention support for removing variables in help text.

Also introduce and consistently reuse 'NV' as abbreviation for 'non volatile'.

Signed-off-by: Enrico Jorns <ejo@pengutronix.de>
---
 commands/nv.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/commands/nv.c b/commands/nv.c
index ebef97eae7..4ea6d1774e 100644
--- a/commands/nv.c
+++ b/commands/nv.c
@@ -88,19 +88,19 @@ static int do_nv(int argc, char *argv[])
 }
 
 BAREBOX_CMD_HELP_START(nv)
-BAREBOX_CMD_HELP_TEXT("Add a new non volatile variable named VAR, optionally set to VALUE.")
-BAREBOX_CMD_HELP_TEXT("non volatile variables are persistent variables that overwrite the")
+BAREBOX_CMD_HELP_TEXT("Add a new non volatile (NV) variable named VAR, optionally set to VALUE.")
+BAREBOX_CMD_HELP_TEXT("NV variables are persistent variables that overwrite the")
 BAREBOX_CMD_HELP_TEXT("global variables of the same name. Their value is saved implicitly with")
 BAREBOX_CMD_HELP_TEXT("'saveenv' or explicitly with 'nv -s'")
 BAREBOX_CMD_HELP_TEXT("")
 BAREBOX_CMD_HELP_TEXT("Options:")
-BAREBOX_CMD_HELP_OPT("-r", "remove non volatile variables")
-BAREBOX_CMD_HELP_OPT("-s", "Save NV variables")
+BAREBOX_CMD_HELP_OPT("-r VAR1 ...", "remove NV variable(s)")
+BAREBOX_CMD_HELP_OPT("-s\t", "save NV variables")
 BAREBOX_CMD_HELP_END
 
 BAREBOX_CMD_START(nv)
 	.cmd		= do_nv,
-	BAREBOX_CMD_DESC("create or set non volatile variables")
+	BAREBOX_CMD_DESC("create, set or remove non volatile (NV) variables")
 	BAREBOX_CMD_OPTS("[-r] VAR[=VALUE] ...")
 	BAREBOX_CMD_GROUP(CMD_GRP_ENV)
 	BAREBOX_CMD_HELP(cmd_nv_help)
-- 
2.11.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 6/6] command: nv: rewrap help text lines

[Enrico Jorns]

Just let the help text appear a bit more aligned.

Signed-off-by: Enrico Jorns <ejo@pengutronix.de>
---
 commands/nv.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/commands/nv.c b/commands/nv.c
index 4ea6d1774e..51b855ee4b 100644
--- a/commands/nv.c
+++ b/commands/nv.c
@@ -89,9 +89,9 @@ static int do_nv(int argc, char *argv[])
 
 BAREBOX_CMD_HELP_START(nv)
 BAREBOX_CMD_HELP_TEXT("Add a new non volatile (NV) variable named VAR, optionally set to VALUE.")
-BAREBOX_CMD_HELP_TEXT("NV variables are persistent variables that overwrite the")
-BAREBOX_CMD_HELP_TEXT("global variables of the same name. Their value is saved implicitly with")
-BAREBOX_CMD_HELP_TEXT("'saveenv' or explicitly with 'nv -s'")
+BAREBOX_CMD_HELP_TEXT("NV variables are persistent variables that overwrite the global variables")
+BAREBOX_CMD_HELP_TEXT("of the same name.")
+BAREBOX_CMD_HELP_TEXT("Their value is saved implicitly with 'saveenv' or explicitly with 'nv -s'")
 BAREBOX_CMD_HELP_TEXT("")
 BAREBOX_CMD_HELP_TEXT("Options:")
 BAREBOX_CMD_HELP_OPT("-r VAR1 ...", "remove NV variable(s)")
-- 
2.11.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH v3 0/6] Fixes for nv command handling

[Sascha Hauer]

On Wed, Nov 01, 2017 at 08:27:04AM +0100, Enrico Jorns wrote:
> This series adds a couple of fixes and enhancement in option parsing and error
> handling for the 'nv' command.
> 
> CHANGES:
> 
> * Add missing curly braces in 1/5
> * properly introduce and use NV abbreviation
> * re-align help text

Applied, thanks

Sascha

> 
> Enrico Jorns (6):
>   commands: nv: fail with verbose message if invoked without args
>   commands: nv: call nvvar_print() only if no argument is given
>   commands: nv: assure error code will be returned when an error
>     occurred
>   common: globvar: let nvvar_remove() report non-existing variable
>   commands: nv: adjust command help
>   command: nv: rewrap help text lines
> 
>  commands/nv.c      | 45 +++++++++++++++++++++++++++++----------------
>  common/globalvar.c |  4 +++-
>  2 files changed, 32 insertions(+), 17 deletions(-)
> 
> -- 
> 2.11.0
> 
> 
> _______________________________________________
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox
> 

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox