diff mbox series

[U-Boot,RFC,v4,08/16] hashtable: import/export entries with flags

Message ID 20190717082525.891-9-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
'flags' value of all the entries in hashtable should be preserved across
save/load of U-Boot environment context.
To hold such information in an exported file, its text format is now
expanded as follows:
name:attr=value<sp>
   ...
\0

where "attr" must be a fixed-length(ENV_FLAGS_ATTR_MAX_LEN) string which
complies with a existing format of ".flags" variable and used by
env_attr_lookup().

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
 include/search.h |  1 +
 lib/hashtable.c  | 61 ++++++++++++++++++++++++++++++++++++++++++++----
 2 files changed, 57 insertions(+), 5 deletions(-)

Comments

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

In message <20190717082525.891-9-takahiro.akashi@linaro.org> you wrote:
> 'flags' value of all the entries in hashtable should be preserved across
> save/load of U-Boot environment context.
> To hold such information in an exported file, its text format is now
> expanded as follows:
> name:attr=value<sp>
>    ...
> \0
>
> where "attr" must be a fixed-length(ENV_FLAGS_ATTR_MAX_LEN) string which
> complies with a existing format of ".flags" variable and used by
> env_attr_lookup().

Full NAK here.  This breaks compatibility with exiting code.
The colon is a legal character in variable names, so you cannot use
it to introduce new meanings.

Please extend existing flag handling in a compatible way instead.

Best regards,

Wolfgang Denk
diff mbox series

Patch

diff --git a/include/search.h b/include/search.h
index 9cece695d726..ab0d7ccb5f8c 100644
--- a/include/search.h
+++ b/include/search.h
@@ -133,6 +133,7 @@  extern int hwalk_r(struct hsearch_data *__htab, int (*callback)(ENTRY *));
 #define H_MATCH_REGEX	(1 << 8) /* search for regular expression matches    */
 #define H_MATCH_METHOD	(H_MATCH_IDENT | H_MATCH_SUBSTR | H_MATCH_REGEX)
 #define H_PROGRAMMATIC	(1 << 9) /* indicate that an import is from env_set() */
+#define H_MATCH_FLAGS	(1 << 10) /* search/grep key  = variable flags	     */
 #define H_ORIGIN_FLAGS	(H_INTERACTIVE | H_PROGRAMMATIC)
 
 #endif /* _SEARCH_H_ */
diff --git a/lib/hashtable.c b/lib/hashtable.c
index d6975d9cafc6..3fe1d38c827e 100644
--- a/lib/hashtable.c
+++ b/lib/hashtable.c
@@ -248,6 +248,15 @@  static inline int _compare_and_overwrite_entry(ENTRY item, ACTION action,
 
 		/* Overwrite existing value? */
 		if ((action == ENTER) && (item.data != NULL)) {
+			/* attributes cannot be changed */
+			if (item.flags != htab->table[idx].entry.flags) {
+				debug("attributes cannot be changed %s\n",
+				      item.key);
+				__set_errno(EINVAL);
+				*retval = NULL;
+				return 0;
+			}
+
 			/* check for permission */
 			if (htab->change_ok != NULL && htab->change_ok(
 			    &htab->table[idx].entry, item.data,
@@ -389,6 +398,8 @@  int hsearch_ext(ENTRY item, ACTION action, ENTRY ** retval,
 		htab->table[idx].used = hval;
 		htab->table[idx].entry.context = item.context;
 		htab->table[idx].entry.key = strdup(item.key);
+		/* TODO: can be overwritten by env_flags_init() */
+		htab->table[idx].entry.flags = item.flags;
 		htab->table[idx].entry.data = strdup(item.data);
 		if (!htab->table[idx].entry.key ||
 		    !htab->table[idx].entry.data) {
@@ -402,7 +413,9 @@  int hsearch_ext(ENTRY item, ACTION action, ENTRY ** retval,
 		/* This is a new entry, so look up a possible callback */
 		env_callback_init(&htab->table[idx].entry);
 		/* Also look for flags */
-		env_flags_init(&htab->table[idx].entry);
+		/* TODO: or we may want to add another argument? */
+		if (!item.flags)
+			env_flags_init(&htab->table[idx].entry);
 
 		/* check for permission */
 		if (htab->change_ok != NULL && htab->change_ok(
@@ -596,6 +609,21 @@  static int match_string(int flag, const char *str, const char *pat, void *priv)
 	return 0;
 }
 
+/* TODO: we always need some kind of regex matching */
+static int match_attr(int flag, const char *attr, const char *pat, void *priv)
+{
+	int found, i;
+
+	for (found = 1, i = 0; i < ENV_FLAGS_ATTR_MAX_LEN; i++) {
+		if (pat[i] != '.' && attr[i] != pat[i]) {
+			found = 0;
+			break;
+		}
+	}
+
+	return found;
+}
+
 static int match_entry(ENTRY *ep, int flag,
 		 int argc, char * const argv[])
 {
@@ -617,6 +645,14 @@  static int match_entry(ENTRY *ep, int flag,
 			if (match_string(flag, ep->key, argv[arg], priv))
 				return 1;
 		}
+		if (flag & H_MATCH_FLAGS) {
+			char attr[ENV_FLAGS_ATTR_MAX_LEN + 1];
+
+			env_flags_encode_attr_from_binflags(attr, ep->flags);
+			attr[ENV_FLAGS_ATTR_MAX_LEN] = '\0';
+			if (match_attr(flag, attr, argv[arg], priv))
+				return 1;
+		}
 		if (flag & H_MATCH_DATA) {
 			if (match_string(flag, ep->data, argv[arg], priv))
 				return 1;
@@ -680,6 +716,7 @@  ssize_t hexport_ext(struct hsearch_data *htab, unsigned int ctx,
 					++s;
 				}
 			}
+			totlen += ENV_FLAGS_ATTR_MAX_LEN + 1; /* for :xxx */
 			totlen += 2;	/* for '=' and 'sep' char */
 		}
 	}
@@ -731,6 +768,9 @@  ssize_t hexport_ext(struct hsearch_data *htab, unsigned int ctx,
 		s = list[i]->key;
 		while (*s)
 			*p++ = *s++;
+		*p++ = ':';
+		env_flags_encode_attr_from_binflags(p, list[i]->flags);
+		p += ENV_FLAGS_ATTR_MAX_LEN;
 		*p++ = '=';
 
 		s = list[i]->data;
@@ -829,9 +869,9 @@  int himport_ext(struct hsearch_data *htab, unsigned int ctx,
 		const char *env, size_t size, const char sep, int flag,
 		int crlf_is_lf, int nvars, char * const vars[])
 {
-	char *data, *sp, *dp, *name, *value;
+	char *data, *sp, *dp, *name, *value, *attr;
 	char *localvars[nvars];
-	int i;
+	int flags, i;
 
 	/* Test for correct arguments.  */
 	if (htab == NULL) {
@@ -927,8 +967,10 @@  int himport_ext(struct hsearch_data *htab, unsigned int ctx,
 		}
 
 		/* parse name */
-		for (name = dp; *dp != '=' && *dp && *dp != sep; ++dp)
-			;
+		for (name = dp, attr = NULL; *dp != '=' && *dp && *dp != sep;
+		     ++dp)
+			if (*dp == ':')
+				attr = dp;
 
 		/* deal with "name" and "name=" entries (delete var) */
 		if (*dp == '\0' || *(dp + 1) == '\0' ||
@@ -936,6 +978,8 @@  int himport_ext(struct hsearch_data *htab, unsigned int ctx,
 			if (*dp == '=')
 				*dp++ = '\0';
 			*dp++ = '\0';	/* terminate name */
+			if (attr)
+				*attr = '\0';
 
 			debug("DELETE CANDIDATE: \"%s\"\n", name);
 			if (!drop_var_from_set(name, nvars, localvars))
@@ -947,6 +991,12 @@  int himport_ext(struct hsearch_data *htab, unsigned int ctx,
 			continue;
 		}
 		*dp++ = '\0';	/* terminate name */
+		if (attr) {
+			*attr = '\0';
+			flags = env_parse_flags_to_bin(++attr);
+		} else {
+			flags = 0; /* TODO: default? */
+		}
 
 		/* parse value; deal with escapes */
 		for (value = sp = dp; *dp && (*dp != sep); ++dp) {
@@ -971,6 +1021,7 @@  int himport_ext(struct hsearch_data *htab, unsigned int ctx,
 		/* enter into hash table */
 		e.context = ctx;
 		e.key = name;
+		e.flags = flags;
 		e.data = value;
 
 		hsearch_ext(e, ENTER, &rv, htab, flag);