| Message ID | 1411122471-20087-1-git-send-email-rob.jones@codethink.co.uk |
|---|---|
| State | Accepted |
| Delegated to: | Pablo Neira |
| Headers | show |
On Fri, Sep 19, 2014 at 11:27:51AM +0100, Rob Jones wrote: > Reduce boilerplate code by using __seq_open_private() instead of seq_open() > in xt_match_open() and xt_target_open(). > > Signed-off-by: Rob Jones <rob.jones@codethink.co.uk> > --- > > This patch uses an existing variant of seq_open() to reduce the kernel code > size. Less code to maintain, good. > The only significant variation from the pre-existing code is the fact that > __seq_open_private() calls kzalloc() rather than kmalloc(), which could > conceivably have an impact on timing. I think you can skip trav->class = MTTG_TRAV_INIT; too, now that trav is set to zero. -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Would you like that change made and re-submitted? It's the work of moments to do it. I was only concentrating on not changing the logic so I didn't check on whether and existing initialisation was made redundant. On 23/09/14 15:02, Pablo Neira Ayuso wrote: > On Fri, Sep 19, 2014 at 11:27:51AM +0100, Rob Jones wrote: >> Reduce boilerplate code by using __seq_open_private() instead of seq_open() >> in xt_match_open() and xt_target_open(). >> >> Signed-off-by: Rob Jones <rob.jones@codethink.co.uk> >> --- >> >> This patch uses an existing variant of seq_open() to reduce the kernel code >> size. > > Less code to maintain, good. > >> The only significant variation from the pre-existing code is the fact that >> __seq_open_private() calls kzalloc() rather than kmalloc(), which could >> conceivably have an impact on timing. > > I think you can skip > > trav->class = MTTG_TRAV_INIT; > > too, now that trav is set to zero. > > >
On Tue, Sep 23, 2014 at 04:32:39PM +0100, Rob Jones wrote: > Would you like that change made and re-submitted? It's the work of > moments to do it. Yes please, send us a v2. Thanks. -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Will do. On 23/09/14 16:36, Pablo Neira Ayuso wrote: > On Tue, Sep 23, 2014 at 04:32:39PM +0100, Rob Jones wrote: >> Would you like that change made and re-submitted? It's the work of >> moments to do it. > > Yes please, send us a v2. Thanks. > > >
diff --git a/net/netfilter/x_tables.c b/net/netfilter/x_tables.c index 227aa11..89aa680 100644 --- a/net/netfilter/x_tables.c +++ b/net/netfilter/x_tables.c @@ -1137,22 +1137,11 @@ static const struct seq_operations xt_match_seq_ops = { static int xt_match_open(struct inode *inode, struct file *file) { - struct seq_file *seq; struct nf_mttg_trav *trav; - int ret; - - trav = kmalloc(sizeof(*trav), GFP_KERNEL); - if (trav == NULL) + trav = __seq_open_private(file, &xt_match_seq_ops, sizeof(*trav)); + if (!trav) return -ENOMEM; - ret = seq_open(file, &xt_match_seq_ops); - if (ret < 0) { - kfree(trav); - return ret; - } - - seq = file->private_data; - seq->private = trav; trav->nfproto = (unsigned long)PDE_DATA(inode); return 0; } @@ -1201,22 +1190,11 @@ static const struct seq_operations xt_target_seq_ops = { static int xt_target_open(struct inode *inode, struct file *file) { - struct seq_file *seq; struct nf_mttg_trav *trav; - int ret; - - trav = kmalloc(sizeof(*trav), GFP_KERNEL); - if (trav == NULL) + trav = __seq_open_private(file, &xt_target_seq_ops, sizeof(*trav)); + if (!trav) return -ENOMEM; - ret = seq_open(file, &xt_target_seq_ops); - if (ret < 0) { - kfree(trav); - return ret; - } - - seq = file->private_data; - seq->private = trav; trav->nfproto = (unsigned long)PDE_DATA(inode); return 0; }
Reduce boilerplate code by using __seq_open_private() instead of seq_open() in xt_match_open() and xt_target_open(). Signed-off-by: Rob Jones <rob.jones@codethink.co.uk> --- This patch uses an existing variant of seq_open() to reduce the kernel code size. The only significant variation from the pre-existing code is the fact that __seq_open_private() calls kzalloc() rather than kmalloc(), which could conceivably have an impact on timing. ** I am sending this to the developers' list now. I failed to transcribe the full cc list output by get-maintainer.pl, mea culpa, mea maxima culpa ** net/netfilter/x_tables.c | 30 ++++-------------------------- 1 file changed, 4 insertions(+), 26 deletions(-)