[U-Boot] [PATCH] api: Use hashtable function for API_env_enum

[Emmanuel Vadot]

On Fri, 11 Nov 2016 09:16:46 -0700
Simon Glass <sjg at chromium.org> wrote:

> Hi,
> 
> On 5 November 2016 at 15:27, Emmanuel Vadot <manu at bidouilliste.com> wrote:
> >
> > Since the env is an hashtable, use the hashtable function for the API_ENV_ENUM
> > api call.
> 
> Can you please explain why in your commit message?

 Hi,

 I will send a new version with the why (spoiler: current code doesn't
work).

> >
> > Signed-off-by: Emmanuel Vadot <manu at bidouilliste.com>
> > ---
> >  api/api.c | 60 +++++++++++++++++++++++++++++++-----------------------------
> >  1 file changed, 31 insertions(+), 29 deletions(-)
> >
> > diff --git a/api/api.c b/api/api.c
> > index 8a1433a..c368511 100644
> > --- a/api/api.c
> > +++ b/api/api.c
> > @@ -495,45 +495,47 @@ static int API_env_set(va_list ap)
> >   */
> >  static int API_env_enum(va_list ap)
> >  {
> > -       int i, n;
> > -       char *last, **next;
> > +       int i, buflen;
> > +       char *last, **next, *s;
> > +       ENTRY *match, search;
> > +       static char *var;
> >
> >         last = (char *)va_arg(ap, unsigned long);
> >
> >         if ((next = (char **)va_arg(ap, uintptr_t)) == NULL)
> >                 return API_EINVAL;
> >
> > -       if (last == NULL)
> > -               /* start over */
> > -               *next = ((char *)env_get_addr(0));
> > -       else {
> > -               *next = last;
> > -
> > -               for (i = 0; env_get_char(i) != '\0'; i = n + 1) {
> > -                       for (n = i; env_get_char(n) != '\0'; ++n) {
> > -                               if (n >= CONFIG_ENV_SIZE) {
> > -                                       /* XXX shouldn't we set *next = NULL?? */
> > -                                       return 0;
> > -                               }
> > -                       }
> > -
> > -                       if (envmatch((uchar *)last, i) < 0)
> > -                               continue;
> > -
> > -                       /* try to get next name */
> > -                       i = n + 1;
> > -                       if (env_get_char(i) == '\0') {
> > -                               /* no more left */
> > -                               *next = NULL;
> > -                               return 0;
> > -                       }
> > -
> > -                       *next = ((char *)env_get_addr(i));
> > -                       return 0;
> > +       if (last == NULL) {
> > +               var = NULL;
> > +               i = 0;
> > +       } else {
> > +               var = strdup(last);
> 
> Need to check for failure here.
> 
> > +               s = strchr(var, '=');
> > +               if (s != NULL)
> > +                       *s = 0;
> > +               search.key = var;
> > +               i = hsearch_r(search, FIND, &match, &env_htab, 0);
> > +               if (i == 0) {
> > +                       i = API_EINVAL;
> > +                       goto done;
> >                 }
> >         }
> >
> > +       /* match the next entry after i */
> > +       i = hmatch_r("", i, &match, &env_htab);
> > +       if (i == 0)
> > +               goto done;
> > +       buflen = strlen(match->key) + strlen(match->data) + 2;
> > +       var = realloc(var, buflen);
> > +       snprintf(var, buflen, "%s=%s", match->key, match->data);
> > +       *next = var;
> >         return 0;
> > +
> > +done:
> > +       free(var);
> > +       var = NULL;
> > +       *next = NULL;
> > +       return i;
> >  }
> >
> >  /*
> > --
> > 2.9.2
> >
> 
> Regards,
> SImon


-- 
Emmanuel Vadot <manu at bidouilliste.com> <manu at freebsd.org>