| Message ID | 20161105212720.91569-1-manu@bidouilliste.com |
|---|---|
| State | Accepted |
| Commit | 6215bd4c1fd6bce95072ad123cf1e4af94ab44e2 |
| Delegated to: | Simon Glass |
| Headers | show |
Hi, On 5 November 2016 at 15:27, Emmanuel Vadot <manu@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? > > Signed-off-by: Emmanuel Vadot <manu@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
On Fri, 11 Nov 2016 09:16:46 -0700 Simon Glass <sjg@chromium.org> wrote: > Hi, > > On 5 November 2016 at 15:27, Emmanuel Vadot <manu@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@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
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); + 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; } /*
Since the env is an hashtable, use the hashtable function for the API_ENV_ENUM api call. Signed-off-by: Emmanuel Vadot <manu@bidouilliste.com> --- api/api.c | 60 +++++++++++++++++++++++++++++++----------------------------- 1 file changed, 31 insertions(+), 29 deletions(-)