* [PATCH 0/2] fs: proc: use seq_open_private()
@ 2014-09-12 14:09 Rob Jones
2014-09-12 14:09 ` [PATCH 1/2] fs: proc: use __seq_open_private() Rob Jones
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Rob Jones @ 2014-09-12 14:09 UTC (permalink / raw)
To: linux-kernel; +Cc: akpm, linux-kernel, rob.jones
Use __seq_open_private() to reduce boilerplate code in proc.
This function has been around, undocumented, for years. It can simplify
the set up code for seq file operations.
Signed-off-by: Rob Jones <rob.jones@codethink.co.uk>
Rob Jones (2):
fs: proc: use __seq_open_private()
fs: proc: use __seq_open_private()
fs/proc/task_mmu.c | 42 ++++++++++++++++--------------------------
fs/proc/task_nommu.c | 22 ++++++++--------------
2 files changed, 24 insertions(+), 40 deletions(-)
--
1.7.10.4
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH 1/2] fs: proc: use __seq_open_private() 2014-09-12 14:09 [PATCH 0/2] fs: proc: use seq_open_private() Rob Jones @ 2014-09-12 14:09 ` Rob Jones 2014-09-12 21:54 ` Andrew Morton 2014-09-12 14:09 ` [PATCH 2/2] " Rob Jones 2014-09-12 21:50 ` [PATCH 0/2] fs: proc: use seq_open_private() Andrew Morton 2 siblings, 1 reply; 9+ messages in thread From: Rob Jones @ 2014-09-12 14:09 UTC (permalink / raw) To: linux-kernel; +Cc: akpm, linux-kernel, rob.jones Reduce boilerplate code by using __seq_open_private() instead of seq_open(). Signed-off-by: Rob Jones <rob.jones@codethink.co.uk> --- fs/proc/task_nommu.c | 22 ++++++++-------------- 1 file changed, 8 insertions(+), 14 deletions(-) diff --git a/fs/proc/task_nommu.c b/fs/proc/task_nommu.c index 678455d..b141050 100644 --- a/fs/proc/task_nommu.c +++ b/fs/proc/task_nommu.c @@ -269,20 +269,14 @@ static int maps_open(struct inode *inode, struct file *file, const struct seq_operations *ops) { struct proc_maps_private *priv; - int ret = -ENOMEM; - - priv = kzalloc(sizeof(*priv), GFP_KERNEL); - if (priv) { - priv->pid = proc_pid(inode); - ret = seq_open(file, ops); - if (!ret) { - struct seq_file *m = file->private_data; - m->private = priv; - } else { - kfree(priv); - } - } - return ret; + + priv = __seq_open_private(file, ops, sizeof(*priv)); + if (!priv) + return -ENOMEM; + + priv->pid = proc_pid(inode); + + return 0; } static int pid_maps_open(struct inode *inode, struct file *file) -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] fs: proc: use __seq_open_private() 2014-09-12 14:09 ` [PATCH 1/2] fs: proc: use __seq_open_private() Rob Jones @ 2014-09-12 21:54 ` Andrew Morton 2014-09-15 13:42 ` Rob Jones 2014-09-29 16:02 ` Rob Jones 0 siblings, 2 replies; 9+ messages in thread From: Andrew Morton @ 2014-09-12 21:54 UTC (permalink / raw) To: Rob Jones; +Cc: linux-kernel, linux-kernel On Fri, 12 Sep 2014 15:09:37 +0100 Rob Jones <rob.jones@codethink.co.uk> wrote: > Reduce boilerplate code by using __seq_open_private() instead of seq_open(). http://ozlabs.org/~akpm/mmots/broken-out/fs-proc-task_nommuc-change-maps_open-to-use-__seq_open_private.patch already did this. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] fs: proc: use __seq_open_private() 2014-09-12 21:54 ` Andrew Morton @ 2014-09-15 13:42 ` Rob Jones 2014-09-29 16:02 ` Rob Jones 1 sibling, 0 replies; 9+ messages in thread From: Rob Jones @ 2014-09-15 13:42 UTC (permalink / raw) To: Andrew Morton; +Cc: linux-kernel, linux-kernel On 12/09/14 22:54, Andrew Morton wrote: > On Fri, 12 Sep 2014 15:09:37 +0100 Rob Jones <rob.jones@codethink.co.uk> wrote: > >> Reduce boilerplate code by using __seq_open_private() instead of seq_open(). > > http://ozlabs.org/~akpm/mmots/broken-out/fs-proc-task_nommuc-change-maps_open-to-use-__seq_open_private.patch > already did this. > Great. Too recent to be in my source tree though, I'm afraid. Even so, it's nice to see that his code is identical to mine (except for the assign-on-declaration, I prefer the whitespace, but hey). I wonder if this was triggered by my documentation patch for the routine. If so, that makes me happy :-) -- Rob Jones Codethink Ltd mailto:rob.jones@codethink.co.uk tel:+44 161 236 5575 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] fs: proc: use __seq_open_private() 2014-09-12 21:54 ` Andrew Morton 2014-09-15 13:42 ` Rob Jones @ 2014-09-29 16:02 ` Rob Jones 1 sibling, 0 replies; 9+ messages in thread From: Rob Jones @ 2014-09-29 16:02 UTC (permalink / raw) To: Andrew Morton; +Cc: linux-kernel, linux-kernel On 12/09/14 22:54, Andrew Morton wrote: > On Fri, 12 Sep 2014 15:09:37 +0100 Rob Jones <rob.jones@codethink.co.uk> wrote: > >> Reduce boilerplate code by using __seq_open_private() instead of seq_open(). > > http://ozlabs.org/~akpm/mmots/broken-out/fs-proc-task_nommuc-change-maps_open-to-use-__seq_open_private.patch > already did this. This duplicated only half of my submission, patch 2/2 did a similar operation on a different file (fs/proc/task_mmu.c). https://lkml.org/lkml/2014/9/12/347 Is that one actually going in (with Andrew's rename to include the file name) or should I resubmit it on its own? -- Rob Jones Codethink Ltd mailto:rob.jones@codethink.co.uk tel:+44 161 236 5575 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/2] fs: proc: use __seq_open_private() 2014-09-12 14:09 [PATCH 0/2] fs: proc: use seq_open_private() Rob Jones 2014-09-12 14:09 ` [PATCH 1/2] fs: proc: use __seq_open_private() Rob Jones @ 2014-09-12 14:09 ` Rob Jones 2014-09-12 21:50 ` [PATCH 0/2] fs: proc: use seq_open_private() Andrew Morton 2 siblings, 0 replies; 9+ messages in thread From: Rob Jones @ 2014-09-12 14:09 UTC (permalink / raw) To: linux-kernel; +Cc: akpm, linux-kernel, rob.jones Reduce boilerplate code by using __seq_open_private() instead of seq_open(). Signed-off-by: Rob Jones <rob.jones@codethink.co.uk> --- fs/proc/task_mmu.c | 42 ++++++++++++++++-------------------------- 1 file changed, 16 insertions(+), 26 deletions(-) diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c index 442177b..c8e9045 100644 --- a/fs/proc/task_mmu.c +++ b/fs/proc/task_mmu.c @@ -235,19 +235,14 @@ static int do_maps_open(struct inode *inode, struct file *file, const struct seq_operations *ops) { struct proc_maps_private *priv; - int ret = -ENOMEM; - priv = kzalloc(sizeof(*priv), GFP_KERNEL); - if (priv) { - priv->pid = proc_pid(inode); - ret = seq_open(file, ops); - if (!ret) { - struct seq_file *m = file->private_data; - m->private = priv; - } else { - kfree(priv); - } - } - return ret; + + priv = __seq_open_private(file, ops, sizeof(*priv)); + if (!priv) + return -ENOMEM; + + priv->pid = proc_pid(inode); + + return 0; } static void @@ -1495,19 +1490,14 @@ static int numa_maps_open(struct inode *inode, struct file *file, const struct seq_operations *ops) { struct numa_maps_private *priv; - int ret = -ENOMEM; - priv = kzalloc(sizeof(*priv), GFP_KERNEL); - if (priv) { - priv->proc_maps.pid = proc_pid(inode); - ret = seq_open(file, ops); - if (!ret) { - struct seq_file *m = file->private_data; - m->private = priv; - } else { - kfree(priv); - } - } - return ret; + + priv = __seq_open_private(file, ops, sizeof(*priv)); + if (!priv) + return -ENOMEM; + + priv->proc_maps.pid = proc_pid(inode); + + return 0; } static int pid_numa_maps_open(struct inode *inode, struct file *file) -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 0/2] fs: proc: use seq_open_private() 2014-09-12 14:09 [PATCH 0/2] fs: proc: use seq_open_private() Rob Jones 2014-09-12 14:09 ` [PATCH 1/2] fs: proc: use __seq_open_private() Rob Jones 2014-09-12 14:09 ` [PATCH 2/2] " Rob Jones @ 2014-09-12 21:50 ` Andrew Morton 2014-09-15 7:21 ` Rob Jones 2 siblings, 1 reply; 9+ messages in thread From: Andrew Morton @ 2014-09-12 21:50 UTC (permalink / raw) To: Rob Jones; +Cc: linux-kernel, linux-kernel On Fri, 12 Sep 2014 15:09:36 +0100 Rob Jones <rob.jones@codethink.co.uk> wrote: > fs: proc: use __seq_open_private() > fs: proc: use __seq_open_private() See the problem? We have two different patches, both named the same. I renamed them to fs/proc/task_nommu.c: use __seq_open_private() fs/proc/task_mmu.c: use __seq_open_private() I really don't understand this practice of replacing "/" with ": " in patch titles. Why not just use the "/"? ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/2] fs: proc: use seq_open_private() 2014-09-12 21:50 ` [PATCH 0/2] fs: proc: use seq_open_private() Andrew Morton @ 2014-09-15 7:21 ` Rob Jones 2014-09-15 7:27 ` Andrew Morton 0 siblings, 1 reply; 9+ messages in thread From: Rob Jones @ 2014-09-15 7:21 UTC (permalink / raw) To: Andrew Morton; +Cc: linux-kernel, linux-kernel On 12/09/14 22:50, Andrew Morton wrote: > On Fri, 12 Sep 2014 15:09:36 +0100 Rob Jones <rob.jones@codethink.co.uk> wrote: > >> fs: proc: use __seq_open_private() >> fs: proc: use __seq_open_private() > > See the problem? We have two different patches, both named the same. Always another gotcha! :-) Seriously, does it say anywhere that patch names have to be unique? It makes perfect sense when it's pointed out but it never occurred to me. I'll make sure I don't do it again. > > I renamed them to > > fs/proc/task_nommu.c: use __seq_open_private() > fs/proc/task_mmu.c: use __seq_open_private() Thank you, much appreciated. I would have been happy to re-submit. > > I really don't understand this practice of replacing "/" with ": " in > patch titles. Why not just use the "/"? I'll do this in future too. Sigh. So much to learn. -- Rob Jones Codethink Ltd mailto:rob.jones@codethink.co.uk tel:+44 161 236 5575 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/2] fs: proc: use seq_open_private() 2014-09-15 7:21 ` Rob Jones @ 2014-09-15 7:27 ` Andrew Morton 0 siblings, 0 replies; 9+ messages in thread From: Andrew Morton @ 2014-09-15 7:27 UTC (permalink / raw) To: Rob Jones; +Cc: linux-kernel, linux-kernel On Mon, 15 Sep 2014 08:21:45 +0100 Rob Jones <rob.jones@codethink.co.uk> wrote: > On 12/09/14 22:50, Andrew Morton wrote: > > On Fri, 12 Sep 2014 15:09:36 +0100 Rob Jones <rob.jones@codethink.co.uk> wrote: > > > >> fs: proc: use __seq_open_private() > >> fs: proc: use __seq_open_private() > > > > See the problem? We have two different patches, both named the same. > > Always another gotcha! :-) > > Seriously, does it say anywhere that patch names have to be unique? It > makes perfect sense when it's pointed out but it never occurred to me. Not explicitly as far as I know. Documentation/SubmittingPatches implies it. Search for "unique". ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2014-09-29 16:02 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2014-09-12 14:09 [PATCH 0/2] fs: proc: use seq_open_private() Rob Jones 2014-09-12 14:09 ` [PATCH 1/2] fs: proc: use __seq_open_private() Rob Jones 2014-09-12 21:54 ` Andrew Morton 2014-09-15 13:42 ` Rob Jones 2014-09-29 16:02 ` Rob Jones 2014-09-12 14:09 ` [PATCH 2/2] " Rob Jones 2014-09-12 21:50 ` [PATCH 0/2] fs: proc: use seq_open_private() Andrew Morton 2014-09-15 7:21 ` Rob Jones 2014-09-15 7:27 ` Andrew Morton
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).