Re: [Linux-kernel-mentees] [PATCH] checkpatch: add warning for avoiding .L prefix symbols in assembly files

[Aditya <yashsri421@gmail.com>]

On 16/1/21 6:13 pm, Dwaipayan Ray wrote:
> On Sat, Jan 16, 2021 at 5:52 PM Aditya Srivastava <yashsri421@gmail.com> wrote:
>>
>> Local symbols prefixed with '.L' do not emit symbol table entries, as
>> they have special meaning for the assembler.
>>
>> '.L' prefixed symbols can be used within a code region, but should be
>> avoided for denoting a range of code via 'SYM_*_START/END' annotations.
>>
>> Add a new check to emit warning on finding the usage of '.L' symbols
>> in '.S' files.
>>
>> Suggested-by: Mark Brown <broonie@kernel.org>
>> Link: https://lore.kernel.org/lkml/20210112210154.GI4646@sirena.org.uk/
>> Signed-off-by: Aditya Srivastava <yashsri421@gmail.com>
>> ---
>>  scripts/checkpatch.pl | 6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
>> index 7030c4d6d126..87d96a039e64 100755
>> --- a/scripts/checkpatch.pl
>> +++ b/scripts/checkpatch.pl
>> @@ -3590,6 +3590,12 @@ sub process {
>>                         }
>>                 }
>>
>> +# check for .L prefix local symbols in .S files
>> +               if ($realfile =~ /\.S$/ && $line =~ /\.L\S+/) {
>> +                       WARN("AVOID_L_PREFIX",
>> +                            "Avoid using '.L' prefixed local symbol names for denoting a range of code via 'SYM_*_START/END' annotations; see Documentation/asm-annotations.rst\n" . $herecurr);
>> +               }
>> +
>>  # check we are in a valid source file C or perl if not then ignore this hunk
>>                 next if ($realfile !~ /\.(h|c|pl|dtsi|dts)$/);
>>
>> --
>> 2.17.1
> 
> From an earlier conversation:
>>   So basically, you can use an .L symbol *inside* a function or a code
>>   segment, you just can't use the .L symbol to contain the code using a
>>   SYM_*_START/END annotation pair.
> 
> So this check warns on all uses of the .L prefix
> I think that might be incorrect.
> 

Hey Dwaipayan,
I think you missed this:

> - If the line contains ".L" prefixed symbol, give user a
> warning/check, so that they can ensure that the line is not inside
> START/END block. (As we may not be able to make sure about the same,
> if the START/END line is not in the patch; otherwise we could run a
> while loop)

At best, I think, we could use $context_function, which should work
for patches, but again, this will not work for files.

Do you have any suggestions?

Thanks
Aditya
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees