linux-kernel - Re: [PATCH] signal: clean up codestyle

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20200902084745.zhwqo56ch6imc6cy@wittgenstein>
Date:   Wed, 2 Sep 2020 10:47:45 +0200
From:   Christian Brauner <christian.brauner@...ntu.com>
To:     linmiaohe <linmiaohe@...wei.com>
Cc:     Oleg Nesterov <oleg@...hat.com>,
        "axboe@...nel.dk" <axboe@...nel.dk>,
        "ebiederm@...ssion.com" <ebiederm@...ssion.com>,
        "madhuparnabhowmik10@...il.com" <madhuparnabhowmik10@...il.com>,
        "gustavoars@...nel.org" <gustavoars@...nel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] signal: clean up codestyle

On Wed, Sep 02, 2020 at 01:34:59AM +0000, linmiaohe wrote:
> Christian Brauner <christian.brauner@...ntu.com> wrote:
> >On Tue, Sep 01, 2020 at 06:39:05PM +0200, Oleg Nesterov wrote:
> >> On 09/01, Christian Brauner wrote:
> >> >
> >> > On Tue, Sep 01, 2020 at 07:58:00AM -0400, Miaohe Lin wrote:
> >> > > No functional change intended.
> >> >
> >> > Hey Miaohe,
> >> >
> >> > Thank you for the patch.
> >> > I'm sure this is well-intended but afaict the whole file has more or 
> >> > less a consistent style already where e.g. sig-1 without spaces 
> >> > seems to be preferred. The same for the casts where most places use 
> >> > a single space.
> >> >
> >> > Now, I know CodingStyle.rst is on your side at least when it comes 
> >> > to the first point:
> >> >
> >> > Use one space around (on each side of) most binary and ternary 
> >> > operators, such as any of these::
> >> >
> >> > 	=  +  -  <  >  *  /  %  |  &  ^  <=  >=  ==  !=  ?  :
> >> >
> >> > but then you'd need to change each place in kernel/signal.c where 
> >> > that is currently not the case.
> >> 
> >> Or simply leave this code alone ;)
> >
> >I was trying to imply that by pointing out that this would be file-global change. I was likely too subtle. ;)
> >
> >Christian
> 
> Sorry for I did not get the imply.

No need to apologize. That's my bad. 

Maybe some context is useful.
One of the reasons why we tend to sometimes not take changes such as
this even though they would be covered by our officially documented
coding style is to keep the churn minimal.
Whenever functional change happens in codepaths such as this the risk of
regressions is quite high. That's partially because we could use more
tests to catch them (And if you're interested in stuff like this then
writing selftests is always great. We can always use more of them.) but
also simply because the code is complex. Having a lot of non-functional
commits that don't really improve the legibility of the code
significantly can become an issue for maintainers. Personally, I tend to
be less worried about this but this is a collaborative endeavour. :)

Thanks!
Christian