Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20220530090139.3030866-1-masahiroy@kernel.org> <20220530090139.3030866-2-masahiroy@kernel.org>
In-Reply-To: <20220530090139.3030866-2-masahiroy@kernel.org>
From:   Nick Desaulniers <ndesaulniers@google.com>
Date:   Mon, 6 Jun 2022 13:16:22 -0700
Message-ID: <CAKwvOdn4hf1PHsj6MdN1ZeYSpBBBK=EixCSwn88bnPvhY_F0WA@mail.gmail.com>
Subject: Re: [PATCH 2/2] modpost: use fnmatch() to simplify match()
To:     Masahiro Yamada <masahiroy@kernel.org>
Cc:     linux-kbuild@vger.kernel.org, linux-kernel@vger.kernel.org,
        Michal Marek <michal.lkml@markovi.net>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Mon, May 30, 2022 at 2:03 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> Replace the own implementation for wildcard (glob) matching with
> a function call to the library function, fnmatch().
>
> Also, change the return type to 'bool'.
>
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>

Thanks for the patch!
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>

> ---
>
>  scripts/mod/modpost.c | 74 ++++++++-----------------------------------
>  1 file changed, 13 insertions(+), 61 deletions(-)
>
> diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> index c1558bacf717..29d5a841e215 100644
> --- a/scripts/mod/modpost.c
> +++ b/scripts/mod/modpost.c
> @@ -13,6 +13,7 @@
>
>  #define _GNU_SOURCE
>  #include <elf.h>
> +#include <fnmatch.h>
>  #include <stdio.h>
>  #include <ctype.h>
>  #include <string.h>
> @@ -710,29 +711,6 @@ static char *get_modinfo(struct elf_info *info, const char *tag)
>         return get_next_modinfo(info, tag, NULL);
>  }
>
> -/**
> - * Test if string s ends in string sub
> - * return 0 if match
> - **/
> -static int strrcmp(const char *s, const char *sub)
> -{
> -       int slen, sublen;
> -
> -       if (!s || !sub)
> -               return 1;
> -
> -       slen = strlen(s);
> -       sublen = strlen(sub);
> -
> -       if ((slen == 0) || (sublen == 0))
> -               return 1;
> -
> -       if (sublen > slen)
> -               return 1;
> -
> -       return memcmp(s + slen - sublen, sub, sublen);
> -}
> -
>  static const char *sym_name(struct elf_info *elf, Elf_Sym *sym)
>  {
>         if (sym)
> @@ -741,48 +719,22 @@ static const char *sym_name(struct elf_info *elf, Elf_Sym *sym)
>                 return "(unknown)";
>  }
>
> -/* The pattern is an array of simple patterns.
> - * "foo" will match an exact string equal to "foo"
> - * "*foo" will match a string that ends with "foo"
> - * "foo*" will match a string that begins with "foo"
> - * "*foo*" will match a string that contains "foo"
> +/*
> + * Check whether the 'string' argument matches one of the 'patterns',
> + * an array of shell wildcard patterns (glob).
> + *
> + * Return true is there is a match.
>   */
> -static int match(const char *sym, const char * const pat[])
> +static bool match(const char *string, const char *const patterns[])
>  {
> -       const char *p;
> -       while (*pat) {
> -               const char *endp;
> -
> -               p = *pat++;
> -               endp = p + strlen(p) - 1;
> +       const char *pattern;
>
> -               /* "*foo*" */
> -               if (*p == '*' && *endp == '*') {
> -                       char *bare = NOFAIL(strndup(p + 1, strlen(p) - 2));
> -                       char *here = strstr(sym, bare);
> -
> -                       free(bare);
> -                       if (here != NULL)
> -                               return 1;
> -               }
> -               /* "*foo" */
> -               else if (*p == '*') {
> -                       if (strrcmp(sym, p + 1) == 0)
> -                               return 1;
> -               }
> -               /* "foo*" */
> -               else if (*endp == '*') {
> -                       if (strncmp(sym, p, strlen(p) - 1) == 0)
> -                               return 1;
> -               }
> -               /* no wildcards */
> -               else {
> -                       if (strcmp(p, sym) == 0)
> -                               return 1;
> -               }
> +       while ((pattern = *patterns++)) {
> +               if (!fnmatch(pattern, string, 0))
> +                       return true;
>         }
> -       /* no match */
> -       return 0;
> +
> +       return false;
>  }
>
>  /* sections that we do not want to do full section mismatch check on */
> --
> 2.32.0
>


-- 
Thanks,
~Nick Desaulniers