Re: [PATCH] gen_compile_commands: fix overlooked module files

[John Hubbard <jhubbard@nvidia.com>]

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@kernel.org>
>> Cc: Nick Desaulniers <ndesaulniers@google.com>
>> Signed-off-by: John Hubbard <jhubbard@nvidia.com>
>> ---
>>   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