Re: [Outreachy kernel] [PATCH 4/6] Staging: kpc2000: kpc_dma: Use spaces around operators.

[Stefano Brivio <sbrivio@redhat.com>]

On Mon, 30 Mar 2020 07:55:45 -0400
Sam Muhammed <jane.pnx9@gmail.com> wrote:

> On Mon, 2020-03-30 at 03:02 +0200, Stefano Brivio wrote:
> > On Wed, 25 Mar 2020 08:57:30 -0400
> > Sam Muhammed <jane.pnx9@gmail.com> wrote:
> >   
> > > On Wed, 2020-03-25 at 13:36 +0100, Greg Kroah-Hartman wrote:  
> > > > On Wed, Mar 25, 2020 at 04:00:36AM -0400, Sam Muhammed wrote:    
> > > > > On Tue, 2020-03-24 at 22:22 +0100, Julia Lawall wrote:    
> > > > > > 
> > > > > > On Tue, 24 Mar 2020, Sam Muhammed wrote:
> > > > > >     
> > > > > > > Cleanup Checkpatch.pl CHECKs about missing
> > > > > > > spaces around multiple operators.
> > > > > > >
> > > > > > > Signed-off-by: Sam Muhammed <jane.pnx9@gmail.com>
> > > > > > > ---
> > > > > > >  drivers/staging/kpc2000/kpc_dma/fileops.c | 12 ++++++------
> > > > > > >  1 file changed, 6 insertions(+), 6 deletions(-)
> > > > > > >
> > > > > > > diff --git a/drivers/staging/kpc2000/kpc_dma/fileops.c b/drivers/staging/kpc2000/kpc_dma/fileops.c
> > > > > > > index d93097ed781c..e599ae1bdfd0 100644
> > > > > > > --- a/drivers/staging/kpc2000/kpc_dma/fileops.c
> > > > > > > +++ b/drivers/staging/kpc2000/kpc_dma/fileops.c
> > > > > > > @@ -19,7 +19,7 @@ static inline
> > > > > > >  unsigned int  count_pages(unsigned long iov_base, size_t iov_len)
> > > > > > >  {
> > > > > > >  	unsigned long first = (iov_base             & PAGE_MASK) >> PAGE_SHIFT;
> > > > > > > -	unsigned long last  = ((iov_base+iov_len-1) & PAGE_MASK) >> PAGE_SHIFT;
> > > > > > > +	unsigned long last  = ((iov_base + iov_len - 1) & PAGE_MASK) >> PAGE_SHIFT;    
> > > > > > 
> > > > > > It seems that the & operators were lined up before and that is not the
> > > > > > case any more.
> > > > > >     
> > > > > 
> > > > > Yes, why they were aligned anyways?     
> > > > 
> > > > Because it is easier to understand the code that way, don't you think?
> > > >     
> > > My bad, iam not used to see the patterns yes, so aligning the first
> > > statement to the second would be a different change? or since a change
> > > is made anyways they both would go together?   
> > 
> > I think yes, this should be a single patch, because:
> > 
> > 1. realigning the first line is needed only as you edit the second one,
> > 
> > 2. with two patches the alignment would be "broken" after the first one
> >    (this is especially a hard rule with build failures and warnings, but
> >    the concept usually applies to these cases as well)
> > 
> > ...however, why would you ask this question and anyway re-post the
> > series 89 minutes later? Mind that this is a good way to get your
> > patches ignored by some maintainers.
> >   
> 
> But reading through Julia's comment about it and re-considering the
> change, my question was already answered with Julia's comment as of 
> _if i make a change, i should keep the style and not break the
> alignment_
> 
> So i probably shouldn't ask the question from the first place and read
> Julia's comment more carefully. :)

Fine, I really don't mind that part... but it would be nice if you
could answer your question yourself on the list, so that:

1. other applicants benefit from it

2. nobody has to wonder if you're still waiting for an answer (my case
   here)

-- 
Stefano