aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJoe Hershberger <joe.hershberger@ni.com>2012-12-11 22:16:21 -0600
committerTom Rini <trini@ti.com>2012-12-13 11:46:55 -0700
commit7afcf3a55b5f484b3d3442053fae8186a3fb92d7 (patch)
tree1aa2c5143a1bbef6570b03652107a09832a2f2ad
parent3d3b52f2586a8bf1c53496547062594fd4386454 (diff)
env: Refactor apply into change_ok
Move the read of the old value to inside the check function. In some cases it can be avoided all together and at the least the code is only called from one place. Also name the function and the callback to more clearly describe what it does. Pass the ENTRY instead of just the name for direct access to the whole data structure. Pass an enum to the callback that specifies the operation being approved. Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>
-rw-r--r--common/cmd_nvedit.c34
-rw-r--r--common/env_common.c3
-rw-r--r--include/environment.h7
-rw-r--r--include/search.h13
-rw-r--r--lib/hashtable.c70
5 files changed, 72 insertions, 55 deletions
diff --git a/common/cmd_nvedit.c b/common/cmd_nvedit.c
index a8dc9a694..da5689ca6 100644
--- a/common/cmd_nvedit.c
+++ b/common/cmd_nvedit.c
@@ -208,10 +208,20 @@ static int do_env_grep(cmd_tbl_t *cmdtp, int flag,
* overwriting of write-once variables.
*/
-int env_check_apply(const char *name, const char *oldval,
- const char *newval, int flag)
+int env_change_ok(const ENTRY *item, const char *newval, enum env_op op,
+ int flag)
{
int console = -1;
+ const char *name;
+#if !defined(CONFIG_ENV_OVERWRITE) && defined(CONFIG_OVERWRITE_ETHADDR_ONCE) \
+&& defined(CONFIG_ETHADDR)
+ const char *oldval = NULL;
+
+ if (op != env_op_create)
+ oldval = item->data;
+#endif
+
+ name = item->key;
/* Default value for NULL to protect string-manipulating functions */
newval = newval ? : "";
@@ -242,12 +252,12 @@ int env_check_apply(const char *name, const char *oldval,
#endif /* CONFIG_CONSOLE_MUX */
}
+#ifndef CONFIG_ENV_OVERWRITE
/*
* Some variables like "ethaddr" and "serial#" can be set only once and
* cannot be deleted, unless CONFIG_ENV_OVERWRITE is defined.
*/
-#ifndef CONFIG_ENV_OVERWRITE
- if (oldval != NULL && /* variable exists */
+ if (op != env_op_create && /* variable exists */
(flag & H_FORCE) == 0) { /* and we are not forced */
if (strcmp(name, "serial#") == 0 ||
(strcmp(name, "ethaddr") == 0
@@ -265,7 +275,7 @@ int env_check_apply(const char *name, const char *oldval,
* (which will erase all variables prior to calling this),
* we want the baudrate to actually change - for real.
*/
- if (oldval != NULL || /* variable exists */
+ if (op != env_op_create || /* variable exists */
(flag & H_NOCLEAR) == 0) { /* or env is clear */
/*
* Switch to new baudrate if new baudrate is supported
@@ -339,20 +349,6 @@ static int _do_env_set(int flag, int argc, char * const argv[])
}
env_id++;
- /*
- * search if variable with this name already exists
- */
- e.key = name;
- e.data = NULL;
- hsearch_r(e, FIND, &ep, &env_htab, 0);
-
- /*
- * Perform requested checks.
- */
- if (env_check_apply(name, ep ? ep->data : NULL, value, 0)) {
- debug("check function did not approve, refusing\n");
- return 1;
- }
/* Delete only ? */
if (argc < 3 || argv[2] == NULL) {
diff --git a/common/env_common.c b/common/env_common.c
index f22f5b968..a960aa803 100644
--- a/common/env_common.c
+++ b/common/env_common.c
@@ -40,7 +40,7 @@ DECLARE_GLOBAL_DATA_PTR;
#include <env_default.h>
struct hsearch_data env_htab = {
- .apply = env_check_apply,
+ .change_ok = env_change_ok,
};
static uchar __env_get_char_spec(int index)
@@ -162,6 +162,7 @@ void env_relocate(void)
{
#if defined(CONFIG_NEEDS_MANUAL_RELOC)
env_reloc();
+ env_htab.change_ok += gd->reloc_off;
#endif
if (gd->env_valid == 0) {
#if defined(CONFIG_ENV_IS_NOWHERE) || defined(CONFIG_SPL_BUILD)
diff --git a/include/environment.h b/include/environment.h
index e8ab7033b..4b19f32be 100644
--- a/include/environment.h
+++ b/include/environment.h
@@ -188,13 +188,12 @@ int set_default_vars(int nvars, char * const vars[]);
int env_import(const char *buf, int check);
/*
- * Check if variable "name" can be changed from oldval to newval,
- * and if so, apply the changes (e.g. baudrate).
+ * Check if variable "item" can be changed to newval
* When (flag & H_FORCE) is set, it does not print out any error
* message and forces overwriting of write-once variables.
*/
-int env_check_apply(const char *name, const char *oldval,
- const char *newval, int flag);
+int env_change_ok(const ENTRY *item, const char *newval, enum env_op op,
+ int flag);
#endif /* DO_DEPS_ONLY */
diff --git a/include/search.h b/include/search.h
index f5165b0a9..fa00ea1b3 100644
--- a/include/search.h
+++ b/include/search.h
@@ -32,6 +32,12 @@
#define __set_errno(val) do { errno = val; } while (0)
+enum env_op {
+ env_op_create,
+ env_op_delete,
+ env_op_overwrite,
+};
+
/* Action which shall be performed in the call the hsearch. */
typedef enum {
FIND,
@@ -59,14 +65,13 @@ struct hsearch_data {
unsigned int filled;
/*
* Callback function which will check whether the given change for variable
- * "name" from "oldval" to "newval" may be applied or not, and possibly apply
- * such change.
+ * "item" to "newval" may be applied or not, and possibly apply such change.
* When (flag & H_FORCE) is set, it shall not print out any error message and
* shall force overwriting of write-once variables.
.* Must return 0 for approval, 1 for denial.
*/
- int (*apply)(const char *name, const char *oldval,
- const char *newval, int flag);
+ int (*change_ok)(const ENTRY *__item, const char *newval, enum env_op,
+ int flag);
};
/* Create a new hashing table which will at most contain NEL elements. */
diff --git a/lib/hashtable.c b/lib/hashtable.c
index f4d579505..6861a4202 100644
--- a/lib/hashtable.c
+++ b/lib/hashtable.c
@@ -66,12 +66,16 @@
* Instead the interface of all functions is extended to take an argument
* which describes the current status.
*/
+
typedef struct _ENTRY {
int used;
ENTRY entry;
} _ENTRY;
+static void _hdelete(const char *key, struct hsearch_data *htab, ENTRY *ep,
+ int idx);
+
/*
* hcreate()
*/
@@ -259,6 +263,17 @@ static inline int _compare_and_overwrite_entry(ENTRY item, ACTION action,
&& strcmp(item.key, htab->table[idx].entry.key) == 0) {
/* Overwrite existing value? */
if ((action == ENTER) && (item.data != NULL)) {
+ /* check for permission */
+ if (htab->change_ok != NULL && htab->change_ok(
+ &htab->table[idx].entry, item.data,
+ env_op_overwrite, flag)) {
+ debug("change_ok() rejected setting variable "
+ "%s, skipping it!\n", item.key);
+ __set_errno(EPERM);
+ *retval = NULL;
+ return 0;
+ }
+
free(htab->table[idx].entry.data);
htab->table[idx].entry.data = strdup(item.data);
if (!htab->table[idx].entry.data) {
@@ -383,6 +398,17 @@ int hsearch_r(ENTRY item, ACTION action, ENTRY ** retval,
++htab->filled;
+ /* check for permission */
+ if (htab->change_ok != NULL && htab->change_ok(
+ &htab->table[idx].entry, item.data, env_op_create, flag)) {
+ debug("change_ok() rejected setting variable "
+ "%s, skipping it!\n", item.key);
+ _hdelete(item.key, htab, &htab->table[idx].entry, idx);
+ __set_errno(EPERM);
+ *retval = NULL;
+ return 0;
+ }
+
/* return new entry */
*retval = &htab->table[idx].entry;
return 1;
@@ -404,6 +430,18 @@ int hsearch_r(ENTRY item, ACTION action, ENTRY ** retval,
* do that.
*/
+static void _hdelete(const char *key, struct hsearch_data *htab, ENTRY *ep,
+ int idx)
+{
+ /* free used ENTRY */
+ debug("hdelete: DELETING key \"%s\"\n", key);
+ free((void *)ep->key);
+ free(ep->data);
+ htab->table[idx].used = -1;
+
+ --htab->filled;
+}
+
int hdelete_r(const char *key, struct hsearch_data *htab, int flag)
{
ENTRY e, *ep;
@@ -420,19 +458,15 @@ int hdelete_r(const char *key, struct hsearch_data *htab, int flag)
}
/* Check for permission */
- if (htab->apply != NULL &&
- htab->apply(ep->key, ep->data, NULL, flag)) {
+ if (htab->change_ok != NULL &&
+ htab->change_ok(ep, NULL, env_op_delete, flag)) {
+ debug("change_ok() rejected deleting variable "
+ "%s, skipping it!\n", key);
__set_errno(EPERM);
return 0;
}
- /* free used ENTRY */
- debug("hdelete: DELETING key \"%s\"\n", key);
- free((void *)ep->key);
- free(ep->data);
- htab->table[idx].used = -1;
-
- --htab->filled;
+ _hdelete(key, htab, ep, idx);
return 1;
}
@@ -800,24 +834,6 @@ int himport_r(struct hsearch_data *htab,
e.key = name;
e.data = value;
- /* if there is an apply function, check what it has to say */
- if (htab->apply != NULL) {
- debug("searching before calling cb function"
- " for %s\n", name);
- /*
- * Search for variable in existing env, so to pass
- * its previous value to the apply callback
- */
- hsearch_r(e, FIND, &rv, htab, 0);
- debug("previous value was %s\n", rv ? rv->data : "");
- if (htab->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, flag);
if (rv == NULL) {
printf("himport_r: can't insert \"%s=%s\" into hash table\n",