Dear Gerlando Falauto,
> On 04/02/2012 09:06 PM, Marek Vasut wrote:
> > Dear Gerlando Falauto,
> >
> >> When variables explicitly specified on the command line are not present
> >> in the imported env, delete them from the running env.
> >> If the variable is also missing from the running env, issue a warning.
> >>
> >> Signed-off-by: Gerlando Falauto<gerlando.fala...@keymile.com>
> >> ---
> >>
> >> lib/hashtable.c | 48
> >> +++++++++++++++++++++++++++++++++++++++++------- 1 files changed, 41
> >> insertions(+), 7 deletions(-)
> >>
> >> diff --git a/lib/hashtable.c b/lib/hashtable.c
> >> index f3f47de..b3d0b64 100644
> >> --- a/lib/hashtable.c
> >> +++ b/lib/hashtable.c
> >> @@ -607,22 +607,32 @@ ssize_t hexport_r(struct hsearch_data *htab, const
> >> char sep, * himport()
> >>
> >> */
> >>
> >> -/* Check whether variable name is amongst vars[] */
> >> -static int is_var_in_set(const char *name, int nvars, char * const
> >> vars[]) +/*
> >> + * Check whether variable 'name' is amongst vars[],
> >> + * and remove all instances by setting the pointer to NULL
> >> + */
> >> +static int is_var_in_set(const char *name, int nvars, char * vars[])
> >>
> >> {
> >>
> >> int i = 0;
> >>
> >> + int res = 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;
> >> + if (vars[i] == NULL)
> >> + continue;
> >> + /* If we found it, delete all of them */
> >> + if (!strcmp(name, vars[i])) {
> >> + vars[i] = NULL;
> >> + res = 1;
> >> + }
> >>
> >> }
> >>
> >> - debug("Skipping non-listed variable %s\n", name);
> >> + if (!res)
> >> + debug("Skipping non-listed variable %s\n", name);
> >>
> >> - return 0;
> >> + return res;
> >>
> >> }
> >>
> >> /*
> >>
> >> @@ -662,9 +672,11 @@ 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 do_apply)
> >> + int nvars, char * const __vars[], int do_apply)
> >>
> >> {
> >>
> >> char *data, *sp, *dp, *name, *value;
> >>
> >> + char *vars[nvars];
> >> + int i;
> >>
> >> /* Test for correct arguments. */
> >> if (htab == NULL) {
> >>
> >> @@ -681,6 +693,10 @@ int himport_r(struct hsearch_data *htab,
> >>
> >> memcpy(data, env, size);
> >> dp = data;
> >>
> >> + /* make a local copy of the list of variables */
> >> + if (nvars)
> >> + memcpy(vars, __vars, sizeof(__vars[0]) * nvars);
> >
> > Stupid question -- do you need the local copy at all? Why?
>
> Because I need some way to keep track of what variables have already
> been taken into account, and it seemed the easiest way to do it without
> touching the original array (which is BTW passed as a const, at least
> pointer-wise).
> I should have written that in the commit message but I forgot.
>
> I'm not particularly fond of it either, but I'd rather do that than
> overwrite the original array. Not that it's needed afterwards by the
> caller...
> Of course the same information (variables "used") could be tracked in
> some other way (e.g. a bitmask array).
Well won't bitfield suffice then?
> I'm not sure about the binary
> code size, but it would just make things much more complicated to
> read... and it's not like this feature (selective importing) is the core
> of the bootloader, I guess.
>
> We can of course argue whether going through the hassle of deleting a
> variable specified on the command line which is not defined in the
> default/imported env is really the right thing to do (in other words,
> whether the whole patch has the right to exist!), but that's a different
> story. That's why I enqueued it as a separate patch.
Honestly, I'm not in the position to properly argue here because I'm still
making myself familiar with the env part of uboot ;-)
>
> >> +
> >>
> >> if ((flag& H_NOCLEAR) == 0) {
> >>
> >> /* Destroy old hash table if one exists */
> >> debug("Destroy Hash Table: %p table = %p\n", htab,
> >>
> >> @@ -809,6 +825,24 @@ int himport_r(struct hsearch_data *htab,
> >>
> >> debug("INSERT: free(data = %p)\n", data);
> >> free(data);
> >>
> >> + /* process variables which were not considered */
> >> + for (i = 0; i< nvars; i++) {
> >> + if (vars[i] == NULL)
> >> + continue;
> >> + /*
> >> + * All variables which were not deleted from the variable list
> >> + * were not present in the imported env
> >> + * This could mean two things:
> >> + * a) if the variable was present in current env, we delete it
> >> + * b) if the variable was not present in current env, we notify
> >> + * it might be a typo
> >> + */
> >> + if (hdelete_r(vars[i], htab, do_apply) == 0)
> >> + printf("WARNING: '%s' neither in running nor in imported
> >
> > env!\n",
> >
> >> vars[i]); + else
> >> + printf("WARNING: '%s' not in imported env, deleting it!
> >
> > \n", vars[i]);
> >
> >> + }
> >> +
> >>
> >> debug("INSERT: done\n");
> >> return 1; /* everything OK */
> >>
> >> }
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
