Re: [RFC 3/6] objtool: arm64: Adapt the stack frame checks and the section analysis for the arm architecture

From: RaphaÃl Gault
Date: Tue Apr 09 2019 - 12:33:45 EST


Hi,

On 4/9/19 5:27 PM, Julien Thierry wrote:
>
>
> On 09/04/2019 17:24, Mark Rutland wrote:
>> On Tue, Apr 09, 2019 at 06:12:04PM +0200, Peter Zijlstra wrote:
>>>
>>> I'm just doing my initial read-through,.. however
>>>
>>> On Tue, Apr 09, 2019 at 02:52:40PM +0100, Raphael Gault wrote:
>>>> +if (!(sec->sh.sh_flags & SHF_EXECINSTR)
>>>> +&& (strcmp(sec->name, ".altinstr_replacement") || !IGNORE_SHF_EXEC_FLAG))
>>>> continue;
>>>
>>> could you please not format code like that. Operators go at the end of
>>> the line, and continuation should match the indentation of the opening
>>> paren. So the above would look like:
>>>
>>>> +if (!(sec->sh.sh_flags & SHF_EXECINSTR) &&
>>>> + (strcmp(sec->name, ".altinstr_replacement") || !IGNORE_SHF_EXEC_FLAG))
>>>> continue;
>>>
>>> You appear to be doing that quit consistently, and it is against style.

Thank you for these remarks, I will correct this!

>>
>> Raphael, as a heads-up, ./scripts/checkpatch.pl can catch issues like
>> this. You can run it over a list of patches, so for a patch series you
>> can run:
>>
>> $ ./scripts/checkpatch.pl *.patch
>>
>> ... and hopefully most of the output will be reasonable.
>>
>
> For this particular case, checkpatch only warns about it if you pass it
> "--strict" option. So in general it might be useful to include this
> option at least for the first pass at including large pieces of code.
>

Indeed that sounds usefull, thanks,

> Cheers,
>

Cheers,

--
Raphael Gault
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.