[U-Boot] [PATCH v3 3/6] env: add check/apply logic to himport_r()
Marek Vasut
marex at denx.de
Mon Apr 2 22:51:36 CEST 2012
Dear Gerlando Falauto,
> On 04/02/2012 09:01 PM, Marek Vasut wrote:
> > Dear Marek Vasut,
> >
> >> Dear Gerlando Falauto,
> >>
> >>> Change hashtable so that a callback function will decide whether a
> >>> variable can be overwritten, and possibly apply the changes.
> >>>
> >>> So add a new field to struct hsearch_data:
> >>> o "apply" callback function to check whether a variable can be
> >>>
> >>> overwritten, and possibly immediately apply the changes;
> >>> when NULL, no check is performed.
> >>>
> >>> And a new argument to himport_r():
> >>> o "do_apply": whether to call the apply callback function
> >>>
> >>> NOTE: This patch does not change the current behavior.
> >>>
> >>> Signed-off-by: Gerlando Falauto<gerlando.falauto at keymile.com>
> >>> ---
> >>>
> >>> common/cmd_nvedit.c | 3 ++-
> >>> common/env_common.c | 8 +++++---
> >>> include/environment.h | 9 +++++++++
> >>> include/search.h | 14 +++++++++++++-
> >>> lib/hashtable.c | 20 +++++++++++++++++++-
> >>> 5 files changed, 48 insertions(+), 6 deletions(-)
> >>>
> >>> diff --git a/common/cmd_nvedit.c b/common/cmd_nvedit.c
> >>> index 59668a6..fc6165f 100644
> >>> --- a/common/cmd_nvedit.c
> >>> +++ b/common/cmd_nvedit.c
> >>> @@ -201,6 +201,7 @@ static int do_env_grep(cmd_tbl_t *cmdtp, int flag,
> >>>
> >>> * 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 instead.
> >>> + * Also called as a callback function by himport_r().
> >>>
> >>> * Returns 0 in case of success, 1 in case of failure.
> >>> * When (flag& H_FORCE) is set, do not print out any error message
> >>> and
> >>>
> >>> force * overwriting of write-once variables.
> >>> @@ -931,7 +932,7 @@ static int do_env_import(cmd_tbl_t *cmdtp, int
> >>> flag,
> >>>
> >>> }
> >>>
> >>> if (himport_r(&env_htab, addr, size, sep, del ? 0 : H_NOCLEAR,
> >>>
> >>> - 0, NULL) == 0) {
> >>> + 0, NULL, 0 /* do_apply */) == 0) {
> >>>
> >>> error("Environment import failed: errno = %d\n", errno);
> >>> return 1;
> >>>
> >>> }
> >>>
> >>> diff --git a/common/env_common.c b/common/env_common.c
> >>> index a93c062..5558c1c 100644
> >>> --- a/common/env_common.c
> >>> +++ b/common/env_common.c
> >>> @@ -122,7 +122,9 @@ const uchar default_environment[] = {
> >>>
> >>> "\0"
> >>>
> >>> };
> >>>
> >>> -struct hsearch_data env_htab;
> >>> +struct hsearch_data env_htab = {
> >>> + .apply = env_check_apply,
> >>> +};
> >>>
> >>> static uchar __env_get_char_spec(int index)
> >>> {
> >>>
> >>> @@ -183,7 +185,7 @@ void set_default_env(const char *s)
> >>>
> >>> if (himport_r(&env_htab, (char *)default_environment,
> >>>
> >>> sizeof(default_environment), '\0', 0,
> >>>
> >>> - 0, NULL) == 0)
> >>> + 0, NULL, 0 /* do_apply */) == 0)
> >>
> >> Please replace that 0 /*...*/ stuff with simple NULL
> >
> > Oh, it's integer ... so just remove the comment, sorry.
>
> The comment would have been a way to shed some light on one of the 8
> arguments the function takes.
> But if it's against some coding style guideline, I'll simply remove it.
Not really, I don't really care ;-)
>
> >>> error("Environment import failed: errno = %d\n", errno);
> >>>
> >>> gd->flags |= GD_FLG_ENV_READY;
> >>>
> >>> @@ -209,7 +211,7 @@ int env_import(const char *buf, int check)
> >>>
> >>> }
> >>>
> >>> if (himport_r(&env_htab, (char *)ep->data, ENV_SIZE, '\0', 0,
> >>>
> >>> - 0, NULL)) {
> >>> + 0, NULL, 0 /* do_apply */)) {
> >>
> >> DTTO, otherwise looks fine.
> >>
> >>> gd->flags |= GD_FLG_ENV_READY;
> >>> return 1;
> >>>
> >>> }
> >>>
> >>> diff --git a/include/environment.h b/include/environment.h
> >>> index 1ef44f3..236e179 100644
> >>> --- a/include/environment.h
> >>> +++ b/include/environment.h
> >>> @@ -193,6 +193,15 @@ void set_default_env(const char *s);
> >>>
> >>> /* Import from binary representation into hash table */
> >>> 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).
> >>> + * 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);
> >>> +
> >>>
> >>> #endif /* DO_DEPS_ONLY */
> >>>
> >>> #endif /* _ENVIRONMENT_H_ */
> >>>
> >>> diff --git a/include/search.h b/include/search.h
> >>> index 94d75fc..721c8ac 100644
> >>> --- a/include/search.h
> >>> +++ b/include/search.h
> >>> @@ -57,6 +57,16 @@ struct hsearch_data {
> >>>
> >>> struct _ENTRY *table;
> >>> unsigned int size;
> >>> 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.
> >>> + * 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);
> >>>
> >>> };
> >>>
> >>> /* Create a new hashing table which will at most contain NEL
> >>> elements. */
> >>>
> >>> @@ -97,10 +107,12 @@ extern ssize_t hexport_r(struct hsearch_data
> >>> *__htab,
> >>>
> >>> /*
> >>>
> >>> * nvars: length of vars array
> >>> * vars: array of strings (variable names) to import (nvars == 0
> >>> means
> >>>
> >>> all) + * do_apply: whether to call callback function to check the new
> >>> argument, + * and possibly apply changes (false means accept
> >>> everything)
> >>>
> >>> */
> >>>
> >>> extern int himport_r(struct hsearch_data *__htab,
> >>>
> >>> const char *__env, size_t __size, const char __sep,
> >>>
> >>> - int __flag, int nvars, char * const vars[]);
> >>> + int __flag, int nvars, char * const vars[], int do_apply);
> >>>
> >>> /* Flags for himport_r() */
> >>> #define H_NOCLEAR (1<< 0) /* do not clear hash table before
> >>
> >> importing */
> >>
> >>> diff --git a/lib/hashtable.c b/lib/hashtable.c
> >>> index 0610e86..6cfba56 100644
> >>> --- a/lib/hashtable.c
> >>> +++ b/lib/hashtable.c
> >>> @@ -658,7 +658,7 @@ static int is_var_in_set(const char *name, int
> >>> nvars, char * const vars[])
> >>>
> >>> int himport_r(struct hsearch_data *htab,
> >>>
> >>> const char *env, size_t size, const char sep, int flag,
> >>>
> >>> - int nvars, char * const vars[])
> >>> + int nvars, char * const vars[], int do_apply)
> >>>
> >>> {
> >>>
> >>> char *data, *sp, *dp, *name, *value;
> >>>
> >>> @@ -772,6 +772,24 @@ 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 (do_apply&& 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);
> >>> + 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);
> >>> if (rv == NULL) {
> >>>
> >>> printf("himport_r: can't insert \"%s=%s\" into
hash
> >>
> >> table\n",
More information about the U-Boot
mailing list