* [U-Boot] [PATCH RFC 1/4] Groundwork for generalization of env interface
2011-09-30 12:04 [U-Boot] [PATCH RFC 0/4] Reworking of env Gerlando Falauto
@ 2011-09-30 12:04 ` Gerlando Falauto
2011-10-06 21:15 ` Wolfgang Denk
2011-09-30 12:04 ` [U-Boot] [PATCH RFC 2/4] env: check and apply changes on delete/destroy Gerlando Falauto
` (2 subsequent siblings)
3 siblings, 1 reply; 8+ messages in thread
From: Gerlando Falauto @ 2011-09-30 12:04 UTC (permalink / raw)
To: u-boot
Signed-off-by: Gerlando Falauto <gerlando.falauto@keymile.com>
---
common/cmd_nvedit.c | 150 ++++++++++++++++++++++++++++++-------------------
common/env_common.c | 15 ++++-
include/environment.h | 2 +
include/search.h | 13 ++++
lib/hashtable.c | 48 ++++++++++++++++
5 files changed, 169 insertions(+), 59 deletions(-)
diff --git a/common/cmd_nvedit.c b/common/cmd_nvedit.c
index e8b116d..05e365a 100644
--- a/common/cmd_nvedit.c
+++ b/common/cmd_nvedit.c
@@ -194,32 +194,23 @@ static int do_env_grep (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[
#endif
/*
- * Set a new environment variable,
- * or replace or delete an existing one.
+ * Performs consistency checking before setting, replacing,
+ * or deleting an environment variable, then (if successful)
+ * apply the changes to internals so to make them effective.
+ * Code for this function was taken out of _do_env_set(),
+ * which now calls it.
+ * Also called as a callback function by himport_ex().
+ * Returns 0 in case of success, 1 in case of failure.
+ * When (flag & H_FORCE) is set, force overwriting of
+ * write-once variables.
*/
-int _do_env_set (int flag, int argc, char * const argv[])
+int env_check_apply(const char *name, const char *oldval,
+ const char *newval, int flag)
{
bd_t *bd = gd->bd;
- int i, len;
+ int i;
int console = -1;
- char *name, *value, *s;
- ENTRY e, *ep;
-
- name = argv[1];
-
- if (strchr(name, '=')) {
- printf("## Error: illegal character '=' in variable name \"%s\"\n", name);
- return 1;
- }
-
- env_id++;
- /*
- * search if variable with this name already exists
- */
- e.key = name;
- e.data = NULL;
- hsearch_r(e, FIND, &ep, &env_htab);
/* Check for console redirection */
if (strcmp(name, "stdin") == 0)
@@ -230,22 +221,22 @@ int _do_env_set (int flag, int argc, char * const argv[])
console = stderr;
if (console != -1) {
- if (argc < 3) { /* Cannot delete it! */
+ if ((newval == NULL) || (*newval == '\0')) { /* Cannot delete it! */
printf("Can't delete \"%s\"\n", name);
return 1;
}
#ifdef CONFIG_CONSOLE_MUX
- i = iomux_doenv(console, argv[2]);
+ i = iomux_doenv(console, newval);
if (i)
return i;
#else
/* Try assigning specified device */
- if (console_assign(console, argv[2]) < 0)
+ if (console_assign(console, newval) < 0)
return 1;
#ifdef CONFIG_SERIAL_MULTI
- if (serial_assign(argv[2]) < 0)
+ if (serial_assign(newval) < 0)
return 1;
#endif
#endif /* CONFIG_CONSOLE_MUX */
@@ -255,23 +246,27 @@ int _do_env_set (int flag, int argc, char * const argv[])
* Some variables like "ethaddr" and "serial#" can be set only
* once and cannot be deleted; also, "ver" is readonly.
*/
- if (ep) { /* variable exists */
+ if (oldval != NULL) { /* variable exists */
#ifndef CONFIG_ENV_OVERWRITE
- if ((strcmp(name, "serial#") == 0) ||
+ if ( ((flag & H_FORCE) == 0) &&
+ ((strcmp(name, "serial#") == 0) ||
((strcmp(name, "ethaddr") == 0)
#if defined(CONFIG_OVERWRITE_ETHADDR_ONCE) && defined(CONFIG_ETHADDR)
- && (strcmp(ep->data, MK_STR(CONFIG_ETHADDR)) != 0)
+ && (strcmp(oldval, MK_STR(CONFIG_ETHADDR)) != 0)
#endif /* CONFIG_OVERWRITE_ETHADDR_ONCE && CONFIG_ETHADDR */
) ) {
printf("Can't overwrite \"%s\"\n", name);
return 1;
}
#endif
+ }
+ if ((oldval != NULL) /* variable exists */
+ || ((flag & H_NOCLEAR) == 0)) { /* or we are in a scratched-out environment */
/*
* Switch to new baudrate if new baudrate is supported
*/
if (strcmp(name, "baudrate") == 0) {
- int baudrate = simple_strtoul(argv[2], NULL, 10);
+ int baudrate = simple_strtoul(newval, NULL, 10);
int i;
for (i = 0; i < N_BAUDRATES; ++i) {
if (baudrate == baudrate_table[i])
@@ -282,6 +277,10 @@ int _do_env_set (int flag, int argc, char * const argv[])
baudrate);
return 1;
}
+ if (gd->baudrate == baudrate) {
+ /* In case of unchanged baudrate, let's just say it's OK */
+ return 0;
+ }
printf ("## Switch baudrate to %d bps and press ENTER ...\n",
baudrate);
udelay(50000);
@@ -299,6 +298,71 @@ int _do_env_set (int flag, int argc, char * const argv[])
}
}
+ /*
+ * Some variables should be updated when the corresponding
+ * entry in the environment is changed
+ */
+
+ if (strcmp(name, "ipaddr") == 0) {
+ const char *s = newval; /* always use only one arg */
+ char *e;
+ unsigned long addr;
+ bd->bi_ip_addr = 0;
+ for (addr = 0, i = 0; i < 4; ++i) {
+ ulong val = s ? simple_strtoul(s, &e, 10) : 0;
+ addr <<= 8;
+ addr |= (val & 0xFF);
+ if (s) s = (*e) ? e+1 : e;
+ }
+ bd->bi_ip_addr = htonl(addr);
+ return 0;
+ } else if (strcmp(newval, "loadaddr") == 0) {
+ load_addr = simple_strtoul(newval, NULL, 16);
+ return 0;
+ }
+#if defined(CONFIG_CMD_NET)
+ else if (strcmp(newval, "bootfile") == 0) {
+ copy_filename(BootFile, newval, sizeof(BootFile));
+ return 0;
+ }
+#endif
+ return 0;
+}
+
+/*
+ * Set a new environment variable,
+ * or replace or delete an existing one.
+*/
+int _do_env_set (int flag, int argc, char * const argv[])
+{
+ int i, len;
+ char *name, *value, *s;
+ ENTRY e, *ep;
+
+ name = argv[1];
+ value = argv[2];
+
+ if (strchr(name, '=')) {
+ printf("## Error: illegal character '=' in variable name \"%s\"\n", name);
+ return 1;
+ }
+
+ env_id++;
+ /*
+ * search if variable with this name already exists
+ */
+ e.key = name;
+ e.data = NULL;
+ hsearch_r(e, FIND, &ep, &env_htab);
+
+ /* Perform requested checks. Notice how since we are overwriting
+ * a single variable, we need to set H_NOCLEAR */
+ if (env_check_apply(name, ep ? ep->data : NULL, value, H_NOCLEAR))
+ {
+ debug("check function did not approve, refusing\n");
+ return 1;
+ }
+
/* Delete only ? */
if ((argc < 3) || argv[2] == NULL) {
int rc = hdelete_r(name, &env_htab);
@@ -336,34 +400,6 @@ int _do_env_set (int flag, int argc, char * const argv[])
return 1;
}
- /*
- * Some variables should be updated when the corresponding
- * entry in the environment is changed
- */
-
- if (strcmp(name, "ipaddr") == 0) {
- char *s = argv[2]; /* always use only one arg */
- char *e;
- unsigned long addr;
- bd->bi_ip_addr = 0;
- for (addr = 0, i = 0; i < 4; ++i) {
- ulong val = s ? simple_strtoul(s, &e, 10) : 0;
- addr <<= 8;
- addr |= (val & 0xFF);
- if (s) s = (*e) ? e+1 : e;
- }
- bd->bi_ip_addr = htonl(addr);
- return 0;
- } else if (strcmp(argv[1], "loadaddr") == 0) {
- load_addr = simple_strtoul(argv[2], NULL, 16);
- return 0;
- }
-#if defined(CONFIG_CMD_NET)
- else if (strcmp(argv[1], "bootfile") == 0) {
- copy_filename(BootFile, argv[2], sizeof(BootFile));
- return 0;
- }
-#endif
return 0;
}
diff --git a/common/env_common.c b/common/env_common.c
index 19149b5..7e1d824 100644
--- a/common/env_common.c
+++ b/common/env_common.c
@@ -172,6 +172,9 @@ const uchar *env_get_addr (int index)
void set_default_env(const char *s)
{
+ /* By default, do not apply changes as they will eventually
+ * be applied by someone else */
+ apply_cb apply_function = NULL;
if (sizeof(default_environment) > ENV_SIZE) {
puts("*** Error - default environment is too large\n\n");
return;
@@ -183,14 +186,22 @@ void set_default_env(const char *s)
"using default environment\n\n",
s+1);
} else {
+ /* This set_to_default was explicitly asked for
+ * by the user, as opposed to being a recovery
+ * mechanism. Therefore we chack every single
+ * variable and apply changes to the system
+ * right away (e.g. baudrate, console).
+ */
+ apply_function = env_check_apply;
puts(s);
}
} else {
puts("Using default environment\n\n");
}
- if (himport_r(&env_htab, (char *)default_environment,
- sizeof(default_environment), '\0', 0) == 0) {
+ if (himport_ex(&env_htab, (char *)default_environment,
+ sizeof(default_environment), '\0', 0,
+ 0, NULL, apply_function) == 0) {
error("Environment import failed: errno = %d\n", errno);
}
gd->flags |= GD_FLG_ENV_READY;
diff --git a/include/environment.h b/include/environment.h
index 6394a96..02c3488 100644
--- a/include/environment.h
+++ b/include/environment.h
@@ -174,6 +174,8 @@ void set_default_env(const char *s);
/* Import from binary representation into hash table */
int env_import(const char *buf, int check);
+int env_check_apply(const char *name, const char *oldval, const char *newval, int flag);
+
#endif
#endif /* _ENVIRONMENT_H_ */
diff --git a/include/search.h b/include/search.h
index b4edd43..4d5b082 100644
--- a/include/search.h
+++ b/include/search.h
@@ -46,6 +46,11 @@ typedef struct entry {
/* Opaque type for internal use. */
struct _ENTRY;
+/* Callback function to be called for checking whether the given change may
+ * be applied or not. Must return 0 for approval, 1 for denial. */
+typedef int (*apply_cb)(const char *name, const char *oldval,
+ const char *newval, int flag);
+
/*
* Family of hash table handling functions. The functions also
* have reentrant counterparts ending with _r. The non-reentrant
@@ -97,7 +102,15 @@ extern int himport_r(struct hsearch_data *__htab,
const char *__env, size_t __size, const char __sep,
int __flag);
+extern int himport_ex(struct hsearch_data *__htab,
+ const char *__env, size_t __size, const char __sep,
+ int __flag,
+ int __argc, char * const __argv[],
+ apply_cb apply);
+
+
/* Flags for himport_r() */
#define H_NOCLEAR 1 /* do not clear hash table before importing */
+#define H_FORCE 2 /* force overwriting of read-only or write-once variables */
#endif /* search.h */
diff --git a/lib/hashtable.c b/lib/hashtable.c
index 026dbca..b088168 100644
--- a/lib/hashtable.c
+++ b/lib/hashtable.c
@@ -628,6 +628,34 @@ ssize_t hexport_r(struct hsearch_data *htab, const char sep,
int himport_r(struct hsearch_data *htab,
const char *env, size_t size, const char sep, int flag)
{
+ return himport_ex(htab, env, size, sep, flag, 0, NULL, NULL);
+}
+
+/* Check whether variable name is amongst vars[] */
+static int process_var(const char *name, int nvars, char * const vars[])
+{
+ int i = 0;
+ /* No variables specified means process all of them*/
+ if (nvars == 0)
+ return 1;
+
+ for (i = 0; i < nvars; i++) {
+ if (!strcmp(name, vars[i])) {
+ return 1;
+ }
+ }
+ debug ("Skipping non-listed variable %s\n", name);
+ return 0;
+}
+
+
+
+int himport_ex(struct hsearch_data *htab,
+ const char *env, size_t size, const char sep, int flag,
+ int nvars, char * const vars[],
+ int(*apply)(const char *, const char *, const char *, int)
+ )
+{
char *data, *sp, *dp, *name, *value;
/* Test for correct arguments. */
@@ -713,6 +741,8 @@ int himport_r(struct hsearch_data *htab,
*dp++ = '\0'; /* terminate name */
debug("DELETE CANDIDATE: \"%s\"\n", name);
+ if (!process_var(name, nvars, vars))
+ continue;
if (hdelete_r(name, htab) == 0)
debug("DELETE ERROR ##############################\n");
@@ -730,10 +760,28 @@ int himport_r(struct hsearch_data *htab,
*sp++ = '\0'; /* terminate value */
++dp;
+ /* Skip variables which are not supposed to be treated */
+ if (!process_var(name, nvars, vars))
+ continue;
+
/* enter into hash table */
e.key = name;
e.data = value;
+ /* if there is an apply function, check what it has to say */
+ if (apply != NULL)
+ {
+ debug("searching before calling cb function for variable %s\n", name);
+ /* Check for previous value so to provide it to apply callback */
+ hsearch_r(e, FIND, &rv, htab);
+ debug("previous value was %s\n", rv ? rv->data : "");
+ if (apply(name, rv ? rv->data : NULL, value, flag))
+ {
+ debug("callback function refused to set variable %s, skipping it!\n", name);
+ continue;
+ }
+ }
+
hsearch_r(e, ENTER, &rv, htab);
if (rv == NULL) {
printf("himport_r: can't insert \"%s=%s\" into hash table\n",
--
1.7.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* [U-Boot] [PATCH RFC 2/4] env: check and apply changes on delete/destroy
2011-09-30 12:04 [U-Boot] [PATCH RFC 0/4] Reworking of env Gerlando Falauto
2011-09-30 12:04 ` [U-Boot] [PATCH RFC 1/4] Groundwork for generalization of env interface Gerlando Falauto
@ 2011-09-30 12:04 ` Gerlando Falauto
2011-10-06 21:15 ` Wolfgang Denk
2011-09-30 12:04 ` [U-Boot] [PATCH RFC 3/4] env: implement selective "env default" Gerlando Falauto
2011-09-30 12:04 ` [U-Boot] [PATCH RFC 4/4] env: implement "env import -n var[, var...]" Gerlando Falauto
3 siblings, 1 reply; 8+ messages in thread
From: Gerlando Falauto @ 2011-09-30 12:04 UTC (permalink / raw)
To: u-boot
Signed-off-by: Gerlando Falauto <gerlando.falauto@keymile.com>
---
common/cmd_nvedit.c | 2 +-
include/search.h | 6 ++++--
lib/hashtable.c | 19 +++++++++++++------
3 files changed, 18 insertions(+), 9 deletions(-)
diff --git a/common/cmd_nvedit.c b/common/cmd_nvedit.c
index 05e365a..506a251 100644
--- a/common/cmd_nvedit.c
+++ b/common/cmd_nvedit.c
@@ -365,7 +365,7 @@ int _do_env_set (int flag, int argc, char * const argv[])
/* Delete only ? */
if ((argc < 3) || argv[2] == NULL) {
- int rc = hdelete_r(name, &env_htab);
+ int rc = hdelete_r(name, &env_htab, NULL);
return !rc;
}
diff --git a/include/search.h b/include/search.h
index 4d5b082..cb30b67 100644
--- a/include/search.h
+++ b/include/search.h
@@ -68,7 +68,8 @@ struct hsearch_data {
extern int hcreate_r(size_t __nel, struct hsearch_data *__htab);
/* Destroy current internal hashing table. */
-extern void hdestroy_r(struct hsearch_data *__htab);
+extern void hdestroy_r(struct hsearch_data *__htab,
+ apply_cb apply);
/*
* Search for entry matching ITEM.key in internal hash table. If
@@ -93,7 +94,8 @@ extern int hstrstr_r(const char *__match, int __last_idx, ENTRY ** __retval,
struct hsearch_data *__htab);
/* Search and delete entry matching ITEM.key in internal hash table. */
-extern int hdelete_r(const char *__key, struct hsearch_data *__htab);
+extern int hdelete_r(const char *__key, struct hsearch_data *__htab,
+ apply_cb apply);
extern ssize_t hexport_r(struct hsearch_data *__htab,
const char __sep, char **__resp, size_t __size);
diff --git a/lib/hashtable.c b/lib/hashtable.c
index b088168..1b65b43 100644
--- a/lib/hashtable.c
+++ b/lib/hashtable.c
@@ -140,7 +140,8 @@ int hcreate_r(size_t nel, struct hsearch_data *htab)
* be freed and the local static variable can be marked as not used.
*/
-void hdestroy_r(struct hsearch_data *htab)
+void hdestroy_r(struct hsearch_data *htab,
+ int(*apply)(const char *, const char *, const char *, int))
{
int i;
@@ -154,7 +155,10 @@ void hdestroy_r(struct hsearch_data *htab)
for (i = 1; i <= htab->size; ++i) {
if (htab->table[i].used > 0) {
ENTRY *ep = &htab->table[i].entry;
-
+ if (apply != NULL) {
+ /* deletion is always forced */
+ apply(ep->key, ep->data, NULL, H_FORCE);
+ }
free((void *)ep->key);
free(ep->data);
}
@@ -399,7 +403,8 @@ int hsearch_r(ENTRY item, ACTION action, ENTRY ** retval,
* do that.
*/
-int hdelete_r(const char *key, struct hsearch_data *htab)
+int hdelete_r(const char *key, struct hsearch_data *htab,
+ int(*apply)(const char *, const char *, const char *, int))
{
ENTRY e, *ep;
int idx;
@@ -415,7 +420,9 @@ int hdelete_r(const char *key, struct hsearch_data *htab)
/* free used ENTRY */
debug("hdelete: DELETING key \"%s\"\n", key);
-
+ if (apply != NULL) {
+ apply(ep->key, ep->data, NULL, H_FORCE);
+ }
free((void *)ep->key);
free(ep->data);
htab->table[idx].used = -1;
@@ -678,7 +685,7 @@ int himport_ex(struct hsearch_data *htab,
debug("Destroy Hash Table: %p table = %p\n", htab,
htab->table);
if (htab->table)
- hdestroy_r(htab);
+ hdestroy_r(htab, apply);
}
/*
@@ -744,7 +751,7 @@ int himport_ex(struct hsearch_data *htab,
if (!process_var(name, nvars, vars))
continue;
- if (hdelete_r(name, htab) == 0)
+ if (hdelete_r(name, htab, apply) == 0)
debug("DELETE ERROR ##############################\n");
continue;
--
1.7.1
^ permalink raw reply related [flat|nested] 8+ messages in thread