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

From: Aditya
Date: Fri Jan 22 2021 - 16:29:34 EST


On 23/1/21 12:40 am, Joe Perches wrote:
> On Fri, 2021-01-22 at 18:48 +0530, Aditya wrote:
>> On 21/1/21 12:13 am, Joe Perches wrote:
>>> I believe the test should be:
>>>
>>> if ($realfile =~ /\.S$/ &&
>>> $line =~ /^\+\s*SYM_[A-Z]+_(?:START|END)(?:_[A-Z_]+)?\s*\(\s*\.L/) {
>>
>> Joe, with this regex, we won't be able to match
>> "jmp .L_restore
>> SYM_FUNC_END(\name)"
>
> I think that's not an issue.
>
>> which was replaced in this patch in the discussion:
>> https://groups.google.com/g/clang-built-linux/c/-drkmLgu-cU/m/phKe-Tb6CgAJ
>>
>> Here, "jmp .L_restore" was also replaced to fix the error.
>
> Notice that this line was also replaced in the same patch:
>
> #ifdef CONFIG_PREEMPTION
> -SYM_CODE_START_LOCAL_NOALIGN(.L_restore)
> +SYM_CODE_START_LOCAL_NOALIGN(__thunk_restore)
>
>
>> However, if this
>> regex(/^\+\s*SYM_[A-Z]+_(?:START|END)(?:_[A-Z_]+)?\s*\(\s*\.L/) is
>> correct, maybe we don't need to check for $file as we are now checking
>> for just one line.
>>
>> What do you think?
>
> The test I wrote was complete, did not use $file and emits a
> warning on the use of CODE_SYM_START_LOCAL_NOALIGN(.L_restore)
>
> I think it's sufficient.
>

Okay, Thanks.. I will send the modified patch :)

Thanks
Aditya