[U-Boot] [PATCH 05/11] env: Simplify the reverse_strstr() interface

[Simon Glass]

Hi Joe,

On 21 April 2015 at 16:02, Joe Hershberger <joe.hershberger at ni.com> wrote:
> The logic to find the whole matching name was split needlessly between
> the reverse_strstr function and its caller. Fully contain it to make the
> interface for calling it more consistent.
>
> Signed-off-by: Joe Hershberger <joe.hershberger at ni.com>
> ---
>
>  common/env_attr.c | 79 +++++++++++++++++++++++++++++--------------------------
>  1 file changed, 41 insertions(+), 38 deletions(-)
>

You could perhaps add some environment tests in test/ for this
function, or access it through getenv(), etc.

> diff --git a/common/env_attr.c b/common/env_attr.c
> index e791f44..d266142 100644
> --- a/common/env_attr.c
> +++ b/common/env_attr.c
> @@ -109,33 +109,55 @@ int env_attr_walk(const char *attr_list,
>  }
>
>  /*
> - * Search for the last matching string in another string with the option to
> - * start looking at a certain point (i.e. ignore anything beyond that point).
> + * Search for the last exactly matching name in an attribute list
>   */
> -static char *reverse_strstr(const char *searched, const char *search_for,
> -       const char *searched_start)
> +static int reverse_name_search(const char *searched, const char *search_for,
> +       const char **result)
>  {
> -       char *result = NULL;
> +       int result_size = 0;
> +       const char *cur_searched = searched;
> +
> +       if (result)
> +               *result = NULL;
>
>         if (*search_for == '\0')
>                 return (char *)searched;
>
>         for (;;) {
> -               char *match = strstr(searched, search_for);
> -
> -               /*
> -                * Stop looking if no new match is found or looking past the
> -                * searched_start pointer
> -                */
> -               if (match == NULL || (searched_start != NULL &&
> -                   match + strlen(search_for) > searched_start))
> +               const char *match = strstr(cur_searched, search_for);
> +               const char *prevch;
> +               const char *nextch;
> +
> +               /* Stop looking if no new match is found */
> +               if (match == NULL)
>                         break;
>
> -               result = match;
> -               searched = match + 1;
> +               prevch = match - 1;
> +               nextch = match + strlen(search_for);
> +
> +               /* Skip spaces */
> +               while (*prevch == ' ')
> +                       prevch--;
> +               while (*nextch == ' ')
> +                       nextch++;
> +
> +               /* Start looking past the current match so last is found */
> +               cur_searched = match + 1;
> +
> +               /* Check for an exact match */
> +               if (match != searched &&
> +                   *prevch != ENV_ATTR_LIST_DELIM)
> +                       continue;
> +               if (*nextch != ENV_ATTR_SEP &&
> +                   *nextch != ENV_ATTR_LIST_DELIM &&
> +                   *nextch != '\0')
> +                       continue;
> +
> +               *result = match;
> +               result_size = strlen(search_for);
>         }
>
> -       return result;
> +       return result_size;
>  }
>
>  /*
> @@ -145,6 +167,7 @@ static char *reverse_strstr(const char *searched, const char *search_for,
>  int env_attr_lookup(const char *attr_list, const char *name, char *attributes)
>  {
>         const char *entry = NULL;
> +       int entry_len;
>
>         if (!attributes)
>                 /* bad parameter */
> @@ -153,32 +176,12 @@ int env_attr_lookup(const char *attr_list, const char *name, char *attributes)
>                 /* list not found */
>                 return -EINVAL;
>
> -       entry = reverse_strstr(attr_list, name, NULL);
> -       while (entry != NULL) {
> -               const char *prevch = entry - 1;
> -               const char *nextch = entry + strlen(name);
> -
> -               /* Skip spaces */
> -               while (*prevch == ' ')
> -                       prevch--;
> -               while (*nextch == ' ')
> -                       nextch++;
> -
> -               /* check for an exact match */
> -               if ((entry == attr_list ||
> -                    *prevch == ENV_ATTR_LIST_DELIM) &&
> -                   (*nextch == ENV_ATTR_SEP ||
> -                    *nextch == ENV_ATTR_LIST_DELIM ||
> -                    *nextch == '\0'))
> -                       break;
> -
> -               entry = reverse_strstr(attr_list, name, entry);
> -       }
> +       entry_len = reverse_name_search(attr_list, name, &entry);
>         if (entry != NULL) {
>                 int len;
>
>                 /* skip the name */
> -               entry += strlen(name);
> +               entry += entry_len;
>                 /* skip spaces */
>                 while (*entry == ' ')
>                         entry++;
> --
> 1.7.11.5

Regards,
Simon