modpost: use strstarts() to clean up parse_source_files()

[PATCH] modpost: use strstarts() to clean up parse_source_files()

Posted by Masahiro Yamada 1 year ago

No functional changes are intended.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

 scripts/mod/sumversion.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/scripts/mod/sumversion.c b/scripts/mod/sumversion.c
index e79fc40d852f..3dd28b4d0099 100644
--- a/scripts/mod/sumversion.c
+++ b/scripts/mod/sumversion.c
@@ -330,7 +330,7 @@ static int parse_source_files(const char *objfile, struct md4_ctx *md)
 			line++;
 		p = line;
 
-		if (strncmp(line, "source_", sizeof("source_")-1) == 0) {
+		if (strstarts(line, "source_")) {
 			p = strrchr(line, ' ');
 			if (!p) {
 				warn("malformed line: %s\n", line);
@@ -344,7 +344,7 @@ static int parse_source_files(const char *objfile, struct md4_ctx *md)
 			}
 			continue;
 		}
-		if (strncmp(line, "deps_", sizeof("deps_")-1) == 0) {
+		if (strstarts(line, "deps_")) {
 			check_files = 1;
 			continue;
 		}
-- 
2.43.0

Re: [PATCH] modpost: use strstarts() to clean up parse_source_files()

Posted by Nicolas Schier 1 year ago

On Sat 08 Feb 2025 02:50:55 GMT, Masahiro Yamada wrote:
> No functional changes are intended.
> 
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> ---
> 
>  scripts/mod/sumversion.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/scripts/mod/sumversion.c b/scripts/mod/sumversion.c
> index e79fc40d852f..3dd28b4d0099 100644
> --- a/scripts/mod/sumversion.c
> +++ b/scripts/mod/sumversion.c
> @@ -330,7 +330,7 @@ static int parse_source_files(const char *objfile, struct md4_ctx *md)
>  			line++;
>  		p = line;
>  
> -		if (strncmp(line, "source_", sizeof("source_")-1) == 0) {
> +		if (strstarts(line, "source_")) {
>  			p = strrchr(line, ' ');
>  			if (!p) {
>  				warn("malformed line: %s\n", line);
> @@ -344,7 +344,7 @@ static int parse_source_files(const char *objfile, struct md4_ctx *md)
>  			}
>  			continue;
>  		}
> -		if (strncmp(line, "deps_", sizeof("deps_")-1) == 0) {
> +		if (strstarts(line, "deps_")) {
>  			check_files = 1;
>  			continue;
>  		}
> -- 
> 2.43.0
> 

Thanks, looks good to me.

Reviewed-by: Nicolas Schier <nicolas@fjasle.eu>

Do you also want to replace the last strncmp() in 
scripts/mod/modpost.c?

diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index e18ae7dc8140..31468923cdf6 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -670,7 +670,7 @@ static char *get_next_modinfo(struct elf_info *info, const char *tag,
        }
 
        for (p = modinfo; p; p = next_string(p, &size)) {
-               if (strncmp(p, tag, taglen) == 0 && p[taglen] == '=')
+               if (strstarts(p, tag) == 0 && p[taglen] == '=')
                        return p + taglen + 1;
        }
        return NULL;


Kind regards,
Nicolas

Re: [PATCH] modpost: use strstarts() to clean up parse_source_files()

Posted by Masahiro Yamada 12 months ago

On Sat, Feb 8, 2025 at 6:38 PM Nicolas Schier <nicolas@fjasle.eu> wrote:
>
> On Sat 08 Feb 2025 02:50:55 GMT, Masahiro Yamada wrote:
> > No functional changes are intended.
> >
> > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> > ---
> >
> >  scripts/mod/sumversion.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/scripts/mod/sumversion.c b/scripts/mod/sumversion.c
> > index e79fc40d852f..3dd28b4d0099 100644
> > --- a/scripts/mod/sumversion.c
> > +++ b/scripts/mod/sumversion.c
> > @@ -330,7 +330,7 @@ static int parse_source_files(const char *objfile, struct md4_ctx *md)
> >                       line++;
> >               p = line;
> >
> > -             if (strncmp(line, "source_", sizeof("source_")-1) == 0) {
> > +             if (strstarts(line, "source_")) {
> >                       p = strrchr(line, ' ');
> >                       if (!p) {
> >                               warn("malformed line: %s\n", line);
> > @@ -344,7 +344,7 @@ static int parse_source_files(const char *objfile, struct md4_ctx *md)
> >                       }
> >                       continue;
> >               }
> > -             if (strncmp(line, "deps_", sizeof("deps_")-1) == 0) {
> > +             if (strstarts(line, "deps_")) {
> >                       check_files = 1;
> >                       continue;
> >               }
> > --
> > 2.43.0
> >
>
> Thanks, looks good to me.
>
> Reviewed-by: Nicolas Schier <nicolas@fjasle.eu>
>
> Do you also want to replace the last strncmp() in
> scripts/mod/modpost.c?
>
> diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> index e18ae7dc8140..31468923cdf6 100644
> --- a/scripts/mod/modpost.c
> +++ b/scripts/mod/modpost.c
> @@ -670,7 +670,7 @@ static char *get_next_modinfo(struct elf_info *info, const char *tag,
>         }
>
>         for (p = modinfo; p; p = next_string(p, &size)) {
> -               if (strncmp(p, tag, taglen) == 0 && p[taglen] == '=')
> +               if (strstarts(p, tag) == 0 && p[taglen] == '=')
>                         return p + taglen + 1;
>         }
>         return NULL;


I believe you meant:

  if (strstarts(p, tag) && p[taglen] == '=')



I do not think there is a strong reason to do so
because taglen is already calculated a few lines above,
but the compiler may be clever enough to avoid
calling strlen() twice. I did not check the compiler output.





-- 
Best Regards
Masahiro Yamada

Re: [PATCH] modpost: use strstarts() to clean up parse_source_files()

Posted by Nicolas Schier 12 months ago

On Tue, Feb 11, 2025 at 09:43:05AM +0900 Masahiro Yamada wrote:
> On Sat, Feb 8, 2025 at 6:38 PM Nicolas Schier <nicolas@fjasle.eu> wrote:
> >
> > On Sat 08 Feb 2025 02:50:55 GMT, Masahiro Yamada wrote:
> > > No functional changes are intended.
> > >
> > > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> > > ---
> > >
> > >  scripts/mod/sumversion.c | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/scripts/mod/sumversion.c b/scripts/mod/sumversion.c
> > > index e79fc40d852f..3dd28b4d0099 100644
> > > --- a/scripts/mod/sumversion.c
> > > +++ b/scripts/mod/sumversion.c
> > > @@ -330,7 +330,7 @@ static int parse_source_files(const char *objfile, struct md4_ctx *md)
> > >                       line++;
> > >               p = line;
> > >
> > > -             if (strncmp(line, "source_", sizeof("source_")-1) == 0) {
> > > +             if (strstarts(line, "source_")) {
> > >                       p = strrchr(line, ' ');
> > >                       if (!p) {
> > >                               warn("malformed line: %s\n", line);
> > > @@ -344,7 +344,7 @@ static int parse_source_files(const char *objfile, struct md4_ctx *md)
> > >                       }
> > >                       continue;
> > >               }
> > > -             if (strncmp(line, "deps_", sizeof("deps_")-1) == 0) {
> > > +             if (strstarts(line, "deps_")) {
> > >                       check_files = 1;
> > >                       continue;
> > >               }
> > > --
> > > 2.43.0
> > >
> >
> > Thanks, looks good to me.
> >
> > Reviewed-by: Nicolas Schier <nicolas@fjasle.eu>
> >
> > Do you also want to replace the last strncmp() in
> > scripts/mod/modpost.c?
> >
> > diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> > index e18ae7dc8140..31468923cdf6 100644
> > --- a/scripts/mod/modpost.c
> > +++ b/scripts/mod/modpost.c
> > @@ -670,7 +670,7 @@ static char *get_next_modinfo(struct elf_info *info, const char *tag,
> >         }
> >
> >         for (p = modinfo; p; p = next_string(p, &size)) {
> > -               if (strncmp(p, tag, taglen) == 0 && p[taglen] == '=')
> > +               if (strstarts(p, tag) == 0 && p[taglen] == '=')
> >                         return p + taglen + 1;
> >         }
> >         return NULL;
> 
> 
> I believe you meant:
> 
>   if (strstarts(p, tag) && p[taglen] == '=')

ups, yes.

> I do not think there is a strong reason to do so
> because taglen is already calculated a few lines above,
> but the compiler may be clever enough to avoid
> calling strlen() twice. I did not check the compiler output.

yeah,  I was thinking about code (style) consistency, but you're right, it
would not be beneficial.

Kind regards,
Nicolas