linux-kernel - Re: [PATCH] mm: memory: fixed a coding style issue

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 for Android: free password hash cracker in your pocket

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

Message-ID: <20180116093143.GC17351@dhcp22.suse.cz>
Date:   Tue, 16 Jan 2018 10:31:43 +0100
From:   Michal Hocko <mhocko@...nel.org>
To:     Robert Rickett <robert.rickett@...il.com>
Cc:     Andrew Morton <akpm@...ux-foundation.org>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] mm: memory: fixed a coding style issue

And now with a response as a bonus...

On Tue 16-01-18 10:26:20, Michal Hocko wrote:
> [Please do not top post. I also think you didn't intend to drop the CC
> list - restored]
> 
> On Tue 16-01-18 03:13:31, Robert Rickett wrote:
> > Michal,
> > 
> >     Although tiny changes like this are minuscule, they are relevant to the
> > big picture. If developers create a tool that informs contributors of an
> >     error in the code, but then says "I'm aware of the error, but it's not
> > worth fixing", what's the point of programming the tool to warn the end-user
> >     in the first place?i

This tool is mostly to keep code consistent. And there is a certain
value in that, no questions about that. And newly added code should use
it as a guidance in many cases. But changing and already existing code
without touching that area for other useful changes is mostly a code
churn which increases chances of merge conflicts. People should also not
underestimate git blame part. It is really tedious to jump over many
commits just to skip over those that are mostly unrelated. Been there
done that...

> > Why not just say "We want you to use tabs, but it's
> > okay not to if you don't feel like it"? Either way, Thank You for taking
> >     the time to respond and have a great day!

This is not about tabs vs. spaces. It is more about checkpatch giving
you hints which are good to follow but they are not _rules_ to follow
blindly. Like for any change also checkpatch driven ones should have
a reasonable justification. E.g. a better readability etc...

> >                                                                     Robert
> > Rickett
> > 
> > 
> > On Tue, Jan 16, 2018 at 1:46 AM, Michal Hocko <mhocko@...nel.org> wrote:
> > 
> > > On Mon 15-01-18 19:17:12, Robert Donald Rickett wrote:
> > > > This is a patch to the memory.c file that fixes the
> > > > "ERROR: code indent should use tabs where possible"
> > > > found by the checkpatch.pl tool.
> > >
> > > Is this really worth it? The code is not any better readable and it just
> > > adds a churn to the history and makes life of anybody using git blame
> > > slightly more harder. So what is the benefit?
> > >
> > > > Signed-off-by: Robert Donald Rickett <robert.rickett@...il.com>
> > > > ---
> > > >  mm/memory.c | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/mm/memory.c b/mm/memory.c
> > > > index ca5674cbaff2..e9f6e58aa77c 100644
> > > > --- a/mm/memory.c
> > > > +++ b/mm/memory.c
> > > > @@ -1663,7 +1663,7 @@ int zap_vma_ptes(struct vm_area_struct *vma,
> > > unsigned long address,
> > > >               unsigned long size)
> > > >  {
> > > >       if (address < vma->vm_start || address + size > vma->vm_end ||
> > > > -                     !(vma->vm_flags & VM_PFNMAP))
> > > > +                     !(vma->vm_flags & VM_PFNMAP))
> > > >               return -1;
> > > >       zap_page_range_single(vma, address, size, NULL);
> > > >       return 0;
> > > > --
> > > > 2.14.1
> > >
> > > --
> > > Michal Hocko
> > > SUSE Labs
> > >
> 
> -- 
> Michal Hocko
> SUSE Labs

-- 
Michal Hocko
SUSE Labs