diff mbox series

[U-Boot,RFC,v4,12/16] env: extend interfaces to get/set attributes

Message ID 20190717082525.891-13-takahiro.akashi@linaro.org
State RFC
Delegated to: Heinrich Schuchardt
Headers show
Series efi_loader: non-volatile variables support | expand

Commit Message

AKASHI Takahiro July 17, 2019, 8:25 a.m. UTC
With this patch, the following interfaces are extended to accept
an additional argument, flags, to get/set attributes of variables.
This feature will be used to implement UEFI variables' semantics,
in particular, that any value change must be reflected to backing
storage immediately.

Meanwhile, existing "env print," "env set" commands are not
extended for now as there is no strong need for that.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
 cmd/nvedit.c      | 59 ++++++++++++++++++++++++++++++-----------------
 include/exports.h |  5 ++--
 2 files changed, 41 insertions(+), 23 deletions(-)

Comments

Wolfgang Denk July 19, 2019, 8:50 a.m. UTC | #1
Dear AKASHI Takahiro,

In message <20190717082525.891-13-takahiro.akashi@linaro.org> you wrote:
> With this patch, the following interfaces are extended to accept
> an additional argument, flags, to get/set attributes of variables.
> This feature will be used to implement UEFI variables' semantics,
> in particular, that any value change must be reflected to backing
> storage immediately.

NAK, see previous comments.

Best regards,

Wolfgang Denk
diff mbox series

Patch

diff --git a/cmd/nvedit.c b/cmd/nvedit.c
index 9896a4141a2a..70fbb5dd8f2f 100644
--- a/cmd/nvedit.c
+++ b/cmd/nvedit.c
@@ -89,7 +89,7 @@  int get_env_id(void)
  */
 static int env_print_ext(enum env_context ctx, char *name, int flag)
 {
-	char *res = NULL;
+	char *res = NULL, attr[ENV_FLAGS_ATTR_MAX_LEN + 1];
 	ssize_t len;
 
 	if (name) {		/* print a single name */
@@ -98,10 +98,12 @@  static int env_print_ext(enum env_context ctx, char *name, int flag)
 		e.key = name;
 		e.context = ctx;
 		e.data = NULL;
-		hsearch_r(e, FIND, &ep, &env_htab, flag);
+		hsearch_ext(e, FIND, &ep, &env_htab, flag);
 		if (ep == NULL)
 			return 0;
-		len = printf("%s=%s\n", ep->key, ep->data);
+		env_flags_encode_attr_from_binflags(attr, ep->flags);
+		attr[ENV_FLAGS_ATTR_MAX_LEN] = '\0';
+		len = printf("%s:%s=%s\n", ep->key, attr, ep->data);
 		return len;
 	}
 
@@ -233,7 +235,7 @@  DONE:
  * or replace or delete an existing one.
  */
 static int _do_env_set(int flag, int argc, char * const argv[], int env_flag,
-		       enum env_context ctx)
+		       enum env_context ctx, int flags)
 {
 	int   i, len;
 	char  *name, *value, *s;
@@ -241,7 +243,7 @@  static int _do_env_set(int flag, int argc, char * const argv[], int env_flag,
 
 	debug("Initial value for argc=%d\n", argc);
 
-	if (ctx == ENVCTX_UEFI)
+	if (ctx == ENVCTX_UEFI && (env_flag & H_INTERACTIVE))
 		return do_env_set_efi(NULL, flag, argc, argv);
 
 	while (argc > 1 && **(argv + 1) == '-') {
@@ -317,8 +319,9 @@  static int _do_env_set(int flag, int argc, char * const argv[], int env_flag,
 
 	e.key	= name;
 	e.context = ctx;
+	e.flags	= flags;
 	e.data	= value;
-	hsearch_r(e, ENTER, &ep, &env_htab, env_flag);
+	hsearch_ext(e, ENTER, &ep, &env_htab, env_flag);
 	free(value);
 	if (!ep) {
 		printf("## Error inserting \"%s\" variable, errno=%d\n",
@@ -341,7 +344,7 @@  static int _do_env_set(int flag, int argc, char * const argv[], int env_flag,
 }
 
 int env_set_ext(const enum env_context ctx,
-		const char *varname, const char *varvalue)
+		const char *varname, int flags, const char *varvalue)
 {
 	const char * const argv[4] = { "setenv", varname, varvalue, NULL };
 
@@ -351,15 +354,15 @@  int env_set_ext(const enum env_context ctx,
 
 	if (varvalue == NULL || varvalue[0] == '\0')
 		return _do_env_set(0, 2, (char * const *)argv, H_PROGRAMMATIC,
-				   ctx);
+				   ctx, flags);
 	else
 		return _do_env_set(0, 3, (char * const *)argv, H_PROGRAMMATIC,
-				   ctx);
+				   ctx, flags);
 }
 
 int env_set(const char *varname, const char *varvalue)
 {
-	return env_set_ext(ENVCTX_UBOOT, varname, varvalue);
+	return env_set_ext(ENVCTX_UBOOT, varname, 0, varvalue);
 }
 
 /**
@@ -446,11 +449,11 @@  static int do_env_set(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 #if CONFIG_IS_ENABLED(CMD_NVEDIT_EFI)
 	if (argc > 1 && argv[1][0] == '-' && argv[1][1] == 'e')
 		return _do_env_set(flag, --argc, ++argv, H_INTERACTIVE,
-				   ENVCTX_UEFI);
+				   ENVCTX_UEFI, 0);
 	else
 #endif
 
-	return _do_env_set(flag, argc, argv, H_INTERACTIVE, ENVCTX_UBOOT);
+	return _do_env_set(flag, argc, argv, H_INTERACTIVE, ENVCTX_UBOOT, 0);
 }
 
 /*
@@ -528,7 +531,8 @@  int do_env_ask(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 	}
 
 	/* Continue calling setenv code */
-	return _do_env_set(flag, len, local_args, H_INTERACTIVE, ENVCTX_UBOOT);
+	return _do_env_set(flag, len, local_args, H_INTERACTIVE, ENVCTX_UBOOT,
+			   0);
 }
 #endif
 
@@ -712,13 +716,13 @@  static int do_env_edit(cmd_tbl_t *cmdtp, int flag, int argc,
 		const char * const _argv[3] = { "setenv", argv[1], NULL };
 
 		return _do_env_set(0, 2, (char * const *)_argv, H_INTERACTIVE,
-				   ENVCTX_UBOOT);
+				   ENVCTX_UBOOT, 0);
 	} else {
 		const char * const _argv[4] = { "setenv", argv[1], buffer,
 			NULL };
 
 		return _do_env_set(0, 3, (char * const *)_argv, H_INTERACTIVE,
-				   ENVCTX_UBOOT);
+				   ENVCTX_UBOOT, 0);
 	}
 }
 #endif /* CONFIG_CMD_EDITENV */
@@ -729,7 +733,7 @@  static int do_env_edit(cmd_tbl_t *cmdtp, int flag, int argc,
  * return address of storage for that variable,
  * or NULL if not found
  */
-char *env_get_ext(const enum env_context ctx, const char *name)
+char *env_get_ext(const enum env_context ctx, const char *name, int *flags)
 {
 	if (gd->flags & GD_FLG_ENV_READY) { /* after import into hashtable */
 		ENTRY e, *ep;
@@ -739,21 +743,34 @@  char *env_get_ext(const enum env_context ctx, const char *name)
 		e.key	= name;
 		e.context = ctx;
 		e.data	= NULL;
-		hsearch_r(e, FIND, &ep, &env_htab, 0);
+		hsearch_ext(e, FIND, &ep, &env_htab, 0);
+
+		if (ep) {
+			if (flags)
+				*flags = ep->flags;
+
+			return ep->data;
+		}
 
-		return ep ? ep->data : NULL;
+		return NULL;
 	}
 
 	/* restricted capabilities before import */
-	if (env_get_f(name, (char *)(gd->env_buf), sizeof(gd->env_buf)) > 0)
+	if (env_get_f(name, (char *)gd->env_buf, sizeof(gd->env_buf)) > 0) {
+		if (flags)
+			*flags = 0;
+
 		return (char *)(gd->env_buf);
+	}
 
 	return NULL;
 }
 
 char *env_get(const char *name)
 {
-	return env_get_ext(ENVCTX_UBOOT, name);
+	int flags;
+
+	return env_get_ext(ENVCTX_UBOOT, name, &flags);
 }
 
 /*
@@ -1240,7 +1257,7 @@  static int do_env_exists(cmd_tbl_t *cmdtp, int flag, int argc,
 	e.key = argv[1];
 	e.context = ENVCTX_UBOOT;
 	e.data = NULL;
-	hsearch_r(e, FIND, &ep, &env_htab, 0);
+	hsearch_ext(e, FIND, &ep, &env_htab, 0);
 
 	return (ep == NULL) ? 1 : 0;
 }
diff --git a/include/exports.h b/include/exports.h
index 0c39d9f16f3d..8eb31cb33b89 100644
--- a/include/exports.h
+++ b/include/exports.h
@@ -28,9 +28,10 @@  unsigned long simple_strtoul(const char *cp, char **endp, unsigned int base);
 int strict_strtoul(const char *cp, unsigned int base, unsigned long *res);
 enum env_context; /* defined in environment.h */
 char *env_get(const char *name);
-char *env_get_ext(enum env_context, const char *name);
+char *env_get_ext(enum env_context, const char *name, int *flags);
 int env_set(const char *varname, const char *value);
-int env_set_ext(enum env_context, const char *varname, const char *value);
+int env_set_ext(enum env_context, const char *varname, int flags,
+		const char *value);
 long simple_strtol(const char *cp, char **endp, unsigned int base);
 int strcmp(const char *cs, const char *ct);
 unsigned long ustrtoul(const char *cp, char **endp, unsigned int base);