All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Michael Kerrisk (man-pages)" <mtk.manpages@gmail.com>
To: Davidlohr Bueso <davidlohr@hp.com>
Cc: mtk.manpages@gmail.com, Manfred Spraul <manfred@colorfullife.com>,
	Davidlohr Bueso <davidlohr.bueso@hp.com>,
	Martin Schwidefsky <schwidefsky@de.ibm.com>,
	LKML <linux-kernel@vger.kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>,
	KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>,
	gthelen@google.com, aswin@hp.com, linux-mm@kvack.org
Subject: Re: [PATCH 5/4] ipc,shm: minor cleanups
Date: Thu, 24 Apr 2014 07:18:06 +0200	[thread overview]
Message-ID: <53589E8E.1040000@gmail.com> (raw)
In-Reply-To: <1398230745.27667.2.camel@buesod1.americas.hpqcorp.net>

On 04/23/2014 07:25 AM, Davidlohr Bueso 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@hp.com>
>>> ---
>>>  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.

Davidlohr,

So, noting that the minority is less small than we thought, I'll just
add this: I'd have appreciated it if your reply had been less 
dismissive, and you'd actually responded to my concrete point about 
loss of readability.

Cheers,

Michael


-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

WARNING: multiple messages have this Message-ID (diff)
From: "Michael Kerrisk (man-pages)" <mtk.manpages@gmail.com>
To: Davidlohr Bueso <davidlohr@hp.com>
Cc: mtk.manpages@gmail.com, Manfred Spraul <manfred@colorfullife.com>,
	Davidlohr Bueso <davidlohr.bueso@hp.com>,
	Martin Schwidefsky <schwidefsky@de.ibm.com>,
	LKML <linux-kernel@vger.kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>,
	KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>,
	gthelen@google.com, aswin@hp.com, linux-mm@kvack.org
Subject: Re: [PATCH 5/4] ipc,shm: minor cleanups
Date: Thu, 24 Apr 2014 07:18:06 +0200	[thread overview]
Message-ID: <53589E8E.1040000@gmail.com> (raw)
In-Reply-To: <1398230745.27667.2.camel@buesod1.americas.hpqcorp.net>

On 04/23/2014 07:25 AM, Davidlohr Bueso 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@hp.com>
>>> ---
>>>  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.

Davidlohr,

So, noting that the minority is less small than we thought, I'll just
add this: I'd have appreciated it if your reply had been less 
dismissive, and you'd actually responded to my concrete point about 
loss of readability.

Cheers,

Michael


-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

  parent reply	other threads:[~2014-04-24  5:38 UTC|newest]

Thread overview: 88+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-04-21 14:26 [PATCH 0/4] ipc/shm.c: increase the limits for SHMMAX, SHMALL Manfred Spraul
2014-04-21 14:26 ` Manfred Spraul
2014-04-21 14:26 ` [PATCH 1/4] ipc/shm.c: check for ulong overflows in shmat Manfred Spraul
2014-04-21 14:26   ` Manfred Spraul
2014-04-21 14:26   ` [PATCH 2/4] ipc/shm.c: check for overflows of shm_tot Manfred Spraul
2014-04-21 14:26     ` Manfred Spraul
2014-04-21 14:26     ` [PATCH 3/4] ipc/shm.c: check for integer overflow during shmget Manfred Spraul
2014-04-21 14:26       ` Manfred Spraul
2014-04-21 14:26       ` [PATCH 4/4] ipc/shm.c: Increase the defaults for SHMALL, SHMMAX Manfred Spraul
2014-04-21 14:26         ` Manfred Spraul
2014-04-22 18:21         ` Davidlohr Bueso
2014-04-22 18:21           ` Davidlohr Bueso
2014-04-22 18:28         ` Davidlohr Bueso
2014-04-22 18:28           ` Davidlohr Bueso
2014-04-22 20:17         ` Motohiro Kosaki
2014-04-22 20:17           ` Motohiro Kosaki
2014-04-23  5:01         ` Michael Kerrisk (man-pages)
2014-04-23  5:01           ` Michael Kerrisk (man-pages)
2014-04-22 18:19       ` [PATCH 3/4] ipc/shm.c: check for integer overflow during shmget Davidlohr Bueso
2014-04-22 18:19         ` Davidlohr Bueso
2014-04-22 20:16         ` Motohiro Kosaki
2014-04-22 20:16           ` Motohiro Kosaki
2014-04-23  4:59       ` Michael Kerrisk (man-pages)
2014-04-23  4:59         ` Michael Kerrisk (man-pages)
2014-04-22 18:18     ` [PATCH 2/4] ipc/shm.c: check for overflows of shm_tot Davidlohr Bueso
2014-04-22 18:18       ` Davidlohr Bueso
2014-04-22 20:16       ` Motohiro Kosaki
2014-04-22 20:16         ` Motohiro Kosaki
2014-04-23  4:58     ` Michael Kerrisk (man-pages)
2014-04-23  4:58       ` Michael Kerrisk (man-pages)
2014-04-22 18:18   ` [PATCH 1/4] ipc/shm.c: check for ulong overflows in shmat Davidlohr Bueso
2014-04-22 18:18     ` Davidlohr Bueso
2014-04-22 20:15     ` Motohiro Kosaki
2014-04-22 20:15       ` Motohiro Kosaki
2014-04-23  4:58   ` Michael Kerrisk (man-pages)
2014-04-23  4:58     ` Michael Kerrisk (man-pages)
2014-04-21 17:25 ` [PATCH 0/4] ipc/shm.c: increase the limits for SHMMAX, SHMALL Davidlohr Bueso
2014-04-21 17:25   ` Davidlohr Bueso
2014-04-22  4:23   ` Manfred Spraul
2014-04-22  4:23     ` Manfred Spraul
2014-04-22 18:18     ` Davidlohr Bueso
2014-04-22 18:18       ` Davidlohr Bueso
2014-04-23  2:53 ` [PATCH 5/4] ipc,shm: minor cleanups Davidlohr Bueso
2014-04-23  2:53   ` Davidlohr Bueso
2014-04-23  5:07   ` Michael Kerrisk (man-pages)
2014-04-23  5:07     ` Michael Kerrisk (man-pages)
2014-04-23  5:25     ` Davidlohr Bueso
2014-04-23  5:25       ` Davidlohr Bueso
2014-04-23  5:28       ` Michael Kerrisk (man-pages)
2014-04-23  5:28         ` Michael Kerrisk (man-pages)
2014-04-23 22:27       ` Andrew Morton
2014-04-23 22:27         ` Andrew Morton
2014-04-23 22:35         ` Stephen Rothwell
2014-04-24  5:18       ` Michael Kerrisk (man-pages) [this message]
2014-04-24  5:18         ` Michael Kerrisk (man-pages)
2014-04-24 17:21         ` Davidlohr Bueso
2014-04-24 17:21           ` Davidlohr Bueso
2014-04-23 18:18   ` Manfred Spraul
2014-04-23 18:18     ` Manfred Spraul
2014-05-02 13:16 ` [PATCH 0/4] ipc/shm.c: increase the limits for SHMMAX, SHMALL Michael Kerrisk (man-pages)
2014-05-02 13:16   ` Michael Kerrisk (man-pages)
2014-05-06 20:06   ` Davidlohr Bueso
2014-05-06 20:06     ` Davidlohr Bueso
2014-05-06 20:40     ` Michael Kerrisk (man-pages)
2014-05-06 20:40       ` Michael Kerrisk (man-pages)
2014-05-06 22:08       ` Davidlohr Bueso
2014-05-06 22:08         ` Davidlohr Bueso
2014-05-07  5:27         ` Michael Kerrisk (man-pages)
2014-05-07  5:27           ` Michael Kerrisk (man-pages)
2014-05-07 18:22           ` Davidlohr Bueso
2014-05-07 18:22             ` Davidlohr Bueso
2014-05-07 19:17             ` [PATCH v2] ipc,shm: document new limits in the uapi header Davidlohr Bueso
2014-05-07 19:17               ` Davidlohr Bueso
2014-05-09  8:44               ` Michael Kerrisk (man-pages)
2014-05-11 20:46                 ` Davidlohr Bueso
2014-05-11 20:46                   ` Davidlohr Bueso
2014-05-12  7:44                   ` Michael Kerrisk (man-pages)
2014-05-13  1:35                     ` Davidlohr Bueso
2014-05-13  1:35                       ` Davidlohr Bueso
2014-05-13  6:06                       ` Michael Kerrisk (man-pages)
2014-05-06 20:43     ` [PATCH 0/4] ipc/shm.c: increase the limits for SHMMAX, SHMALL Davidlohr Bueso
2014-05-06 20:43       ` Davidlohr Bueso
2014-06-03 19:26   ` Davidlohr Bueso
2014-06-03 19:26     ` Davidlohr Bueso
2014-09-23  5:24     ` Michael Kerrisk (man-pages)
2014-09-23  5:24       ` Michael Kerrisk (man-pages)
2014-09-24  8:02       ` Davidlohr Bueso
2014-09-24  8:02         ` Davidlohr Bueso

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=53589E8E.1040000@gmail.com \
    --to=mtk.manpages@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=aswin@hp.com \
    --cc=davidlohr.bueso@hp.com \
    --cc=davidlohr@hp.com \
    --cc=gthelen@google.com \
    --cc=kamezawa.hiroyu@jp.fujitsu.com \
    --cc=kosaki.motohiro@jp.fujitsu.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=manfred@colorfullife.com \
    --cc=schwidefsky@de.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.