Re: [PATCH] gen_compile_commands: fix overlooked module files

[John Hubbard]

On 6/15/22 02:23, Masahiro Yamada wrote:
> > Fixes: ecca4fea1ede4 ("gen_compile_commands: support *.o, *.a, modules.order in positional argument")
>
> This should be
>
> Fixes: 9413e7640564 ("kbuild: split the second line of *.mod into *.usyms")
>
>
> Can you update the commit log?

Done.

> > Cc: Masahiro Yamada <masahiroy@xxxxxxxxxx>
> > Cc: Nick Desaulniers <ndesaulniers@xxxxxxxxxx>
> > Signed-off-by: John Hubbard <jhubbard@xxxxxxxxxx>
> > ---
> >   scripts/clang-tools/gen_compile_commands.py | 7 ++++---
> >   1 file changed, 4 insertions(+), 3 deletions(-)
> >
> > diff --git a/scripts/clang-tools/gen_compile_commands.py b/scripts/clang-tools/gen_compile_commands.py
> > index 1d1bde1fd45e..53590e886889 100755
> > --- a/scripts/clang-tools/gen_compile_commands.py
> > +++ b/scripts/clang-tools/gen_compile_commands.py
> > @@ -157,10 +157,11 @@ def cmdfiles_for_modorder(modorder):
> >               if ext != '.ko':
> >                   sys.exit('{}: module path must end with .ko'.format(ko))
> >               mod = base + '.mod'
> > -           # The first line of *.mod lists the objects that compose the module.
> > +           # Read from *.mod, to get a list of objects that compose the module.
> >               with open(mod) as m:
> > -                for obj in m.readline().split():
> > -                    yield to_cmdfile(obj)
> > +                for line in m.readlines():
>
>
>                      for line in m:
>
> is simpler, (and maybe will work more efficiently).

aha, yes.

> One more note, the 'line' iterator is shadowing (overwriting)
> the outer 'line' iterator, which has been used a few lines above.

I overlooked that point, good catch.

>      with open(modorder) as f:
>          for line in f:
>
>
>
> Maybe, it is safer to use a different name for the inner iterator
> because shadowing does not work in Python.
>
>
>
>
>
> > +                    for obj in line.split():
>
> This loop is unneeded because each line
> contains only one word.
> .rstpip() will do.
>
>
>
> To sum up, this part can be simpler,
> for example like this:
>
>             # Read from *.mod, to get a list of objects that compose the module.
>              with open(mod) as m:
>                  for line2 in m:
>                      yield to_cmdfile(line2.rstrip())

Yes, I'll send out a v2 with all of this. Maybe "mod_line" or something
instead of "line2", but otherwise as you've recommended.

thanks,
--
John Hubbard
NVIDIA