Re: [PATCH 5/4] ipc,shm: minor cleanups

[Andrew Morton]

On Tue, 22 Apr 2014 22:25:45 -0700 Davidlohr Bueso <davidlohr@xxxxxx> wrote:

> On Wed, 2014-04-23 at 07:07 +0200, Michael Kerrisk (man-pages) wrote:
> > On 04/23/2014 04:53 AM, Davidlohr Bueso wrote:
> > > -  Breakup long function names/args.
> > > -  Cleaup variable declaration.
> > > -  s/current->mm/mm
> > > 
> > > Signed-off-by: Davidlohr Bueso <davidlohr@xxxxxx>
> > > ---
> > >  ipc/shm.c | 40 +++++++++++++++++-----------------------
> > >  1 file changed, 17 insertions(+), 23 deletions(-)
> > > 
> > > diff --git a/ipc/shm.c b/ipc/shm.c
> > > index f000696..584d02e 100644
> > > --- a/ipc/shm.c
> > > +++ b/ipc/shm.c
> > > @@ -480,15 +480,13 @@ static const struct vm_operations_struct shm_vm_ops = {
> > >  static int newseg(struct ipc_namespace *ns, struct ipc_params *params)
> > >  {
> > >  	key_t key = params->key;
> > > -	int shmflg = params->flg;
> > > +	int id, error, shmflg = params->flg;
> > 
> > It's largely a matter of taste (and I may be in a minority), and I know
> > there's certainly precedent in the kernel code, but I don't much like the 
> > style of mixing variable declarations that have initializers, with other
> > unrelated declarations (e.g., variables without initializers). What is 
> > the gain? One less line of text? That's (IMO) more than offset by the 
> > small loss of readability.
> 
> Yes, it's taste. And yes, your in the minority, at least in many core
> kernel components and ipc.

I'm with Michael.

- Putting multiple definitions on the same line (whether or not they
  are initialized there) makes it impossible to add little comments
  documenting them.  And we need more little comments documenting
  locals.

- Having multiple definitions on the same line is maddening when the
  time comes to resolve patch conflicts.  And it increases the
  likelihood of conflicts in the first place.

- It makes it much harder to *find* a definition.


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/