[RFA][PATCH 0/8] seq_file: Remove return checks of seq_printf()

[RFA][PATCH 0/8] seq_file: Remove return checks of seq_printf()

[Steven Rostedt]

[ REQUEST FOR ACKS ]

I'm looking to clean up the seq_file code and to eventually merge the
trace_seq code with seq_file as well, since they basically do the same thing.

Part of this process is to remove the return code of seq_printf() and friends
as they are rather inconsistent. It is better to use the new function
seq_has_overflowed() if you want to stop processing when the buffer
is full. Note, if the buffer is full, the seq_file code will throw away
the contents, allocate a bigger buffer, and then call your code again
to fill in the data. The only thing that breaking out of the function
early does is to save a little time which is probably never noticed.

But anyway, I'm asking for Acked-bys for code that touches the subsystems
that have been modified.

I started with patches from Joe Perches and modified them as well.
There's many more places that need to be updated before we can convert
seq_printf() and friends to return void. But this patch set introduces
the seq_has_overflowed() and does some initial updates.

Please ack!

Thanks!

-- Steve



Joe Perches (6):
      seq_file: Rename seq_overflow() to seq_has_overflowed() and make public
      netfilter: Convert print_tuple functions to return void
      dlm: Remove seq_printf() return checks and use seq_has_overflowed()
      dlm: Use seq_puts() instead of seq_printf() for constant strings
      fs: Convert show_fdinfo functions to void
      debugfs: Have debugfs_print_regs32() return void

Steven Rostedt (Red Hat) (2):
      netfilter: Remove return values for print_conntrack callbacks
      netfilter: Remove checks of seq_printf() return values

----
 Documentation/filesystems/debugfs.txt              |   2 +-
 Documentation/filesystems/seq_file.txt             |  22 +-
 Documentation/filesystems/vfs.txt                  |   2 +-
 drivers/net/tun.c                                  |   4 +-
 fs/debugfs/file.c                                  |  15 +-
 fs/dlm/debug_fs.c                                  | 263 ++++++++++-----------
 fs/eventfd.c                                       |   9 +-
 fs/eventpoll.c                                     |  13 +-
 fs/notify/fdinfo.c                                 |  78 +++---
 fs/notify/fdinfo.h                                 |   4 +-
 fs/proc/fd.c                                       |   3 +-
 fs/seq_file.c                                      |  15 +-
 fs/signalfd.c                                      |   4 +-
 fs/timerfd.c                                       |  27 ++-
 include/linux/debugfs.h                            |   7 +-
 include/linux/fs.h                                 |   2 +-
 include/linux/seq_file.h                           |  15 ++
 include/net/netfilter/nf_conntrack_core.h          |   2 +-
 include/net/netfilter/nf_conntrack_l3proto.h       |   4 +-
 include/net/netfilter/nf_conntrack_l4proto.h       |   6 +-
 net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c     |   6 +-
 .../netfilter/nf_conntrack_l3proto_ipv4_compat.c   |  53 +++--
 net/ipv4/netfilter/nf_conntrack_proto_icmp.c       |  10 +-
 net/ipv6/netfilter/nf_conntrack_l3proto_ipv6.c     |   6 +-
 net/ipv6/netfilter/nf_conntrack_proto_icmpv6.c     |  10 +-
 net/netfilter/nf_conntrack_l3proto_generic.c       |   5 +-
 net/netfilter/nf_conntrack_proto_dccp.c            |  14 +-
 net/netfilter/nf_conntrack_proto_generic.c         |   5 +-
 net/netfilter/nf_conntrack_proto_gre.c             |  18 +-
 net/netfilter/nf_conntrack_proto_sctp.c            |  14 +-
 net/netfilter/nf_conntrack_proto_tcp.c             |  14 +-
 net/netfilter/nf_conntrack_proto_udp.c             |  10 +-
 net/netfilter/nf_conntrack_proto_udplite.c         |  10 +-
 net/netfilter/nf_conntrack_standalone.c            |  77 +++---
 net/netfilter/nf_log.c                             |  30 +--
 net/netfilter/nfnetlink_queue_core.c               |  13 +-
 net/netfilter/x_tables.c                           |  19 +-
 net/netfilter/xt_hashlimit.c                       |  36 ++-
 38 files changed, 410 insertions(+), 437 deletions(-)

[RFA][PATCH 1/8] seq_file: Rename seq_overflow() to seq_has_overflowed() and make public

[Steven Rostedt]

[-- Attachment #1: 0001-seq_file-Rename-seq_overflow-to-seq_has_overflowed-a.patch --]
[-- Type: text/plain, Size: 5197 bytes --]

From: Joe Perches <joe@perches.com>

[ REQUEST FOR ACKS ]

The return values of seq_printf/puts/putc are frequently misused.

Start down a path to remove all the return value uses of these
functions.

Move the seq_overflow() to a global inlined function called seq_has_overflowed()
that can be used by the users of seq_file() calls.

Update the documentation to not show return types for seq_printf
et al.  Add a description of seq_has_overflowed().

Link: http://lkml.kernel.org/p/848ac7e3d1c31cddf638a8526fa3c59fa6fdeb8a.1412031505.git.joe@perches.com

Cc: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Joe Perches <joe@perches.com>
[ Reworked the original patch from Joe ]
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 Documentation/filesystems/seq_file.txt | 22 +++++++++++++---------
 fs/seq_file.c                          | 15 ++-------------
 include/linux/seq_file.h               | 15 +++++++++++++++
 3 files changed, 30 insertions(+), 22 deletions(-)

diff --git a/Documentation/filesystems/seq_file.txt b/Documentation/filesystems/seq_file.txt
index 8ea3e90ace07..79b4037a86e6 100644
--- a/Documentation/filesystems/seq_file.txt
+++ b/Documentation/filesystems/seq_file.txt
@@ -180,23 +180,19 @@ output must be passed to the seq_file code. Some utility functions have
 been defined which make this task easy.
 
 Most code will simply use seq_printf(), which works pretty much like
-printk(), but which requires the seq_file pointer as an argument. It is
-common to ignore the return value from seq_printf(), but a function
-producing complicated output may want to check that value and quit if
-something non-zero is returned; an error return means that the seq_file
-buffer has been filled and further output will be discarded.
+printk(), but which requires the seq_file pointer as an argument.
 
 For straight character output, the following functions may be used:
 
-	int seq_putc(struct seq_file *m, char c);
-	int seq_puts(struct seq_file *m, const char *s);
-	int seq_escape(struct seq_file *m, const char *s, const char *esc);
+	seq_putc(struct seq_file *m, char c);
+	seq_puts(struct seq_file *m, const char *s);
+	seq_escape(struct seq_file *m, const char *s, const char *esc);
 
 The first two output a single character and a string, just like one would
 expect. seq_escape() is like seq_puts(), except that any character in s
 which is in the string esc will be represented in octal form in the output.
 
-There is also a pair of functions for printing filenames:
+There are also a pair of functions for printing filenames:
 
 	int seq_path(struct seq_file *m, struct path *path, char *esc);
 	int seq_path_root(struct seq_file *m, struct path *path,
@@ -209,6 +205,14 @@ root is desired, it can be used with seq_path_root().  Note that, if it
 turns out that path cannot be reached from root, the value of root will be
 changed in seq_file_root() to a root which *does* work.
 
+A function producing complicated output may want to check
+	bool seq_has_overflowed(struct seq_file *m);
+and avoid further seq_<output> calls if true is returned.
+
+A true return from seq_has_overflowed means that the seq_file buffer is full
+and further output will be discarded.  The seq_show function will attempt
+to allocate a larger buffer and retry printing.
+
 
 Making it all work
 
diff --git a/fs/seq_file.c b/fs/seq_file.c
index 3857b720cb1b..353948ba1c5b 100644
--- a/fs/seq_file.c
+++ b/fs/seq_file.c
@@ -16,17 +16,6 @@
 #include <asm/uaccess.h>
 #include <asm/page.h>
 
-
-/*
- * seq_files have a buffer which can may overflow. When this happens a larger
- * buffer is reallocated and all the data will be printed again.
- * The overflow state is true when m->count == m->size.
- */
-static bool seq_overflow(struct seq_file *m)
-{
-	return m->count == m->size;
-}
-
 static void seq_set_overflow(struct seq_file *m)
 {
 	m->count = m->size;
@@ -124,7 +113,7 @@ static int traverse(struct seq_file *m, loff_t offset)
 			error = 0;
 			m->count = 0;
 		}
-		if (seq_overflow(m))
+		if (seq_has_overflowed(m))
 			goto Eoverflow;
 		if (pos + m->count > offset) {
 			m->from = offset - pos;
@@ -267,7 +256,7 @@ Fill:
 			break;
 		}
 		err = m->op->show(m, p);
-		if (seq_overflow(m) || err) {
+		if (seq_has_overflowed(m) || err) {
 			m->count = offs;
 			if (likely(err <= 0))
 				break;
diff --git a/include/linux/seq_file.h b/include/linux/seq_file.h
index 52e0097f61f0..07c98e1998c3 100644
--- a/include/linux/seq_file.h
+++ b/include/linux/seq_file.h
@@ -43,6 +43,21 @@ struct seq_operations {
 #define SEQ_SKIP 1
 
 /**
+ * seq_has_overflowed - check if the buffer associated to seq_file has filled
+ * @m: the seq_file handle
+ *
+ * seq_files have a buffer which may overflow. When this happens a larger
+ * buffer is reallocated and all the data will be printed again.
+ * The overflow state is true when m->count == m->size.
+ *
+ * Returns true if the buffer received more than it can hold.
+ */
+static inline bool seq_has_overflowed(struct seq_file *m)
+{
+	return m->count == m->size;
+}
+
+/**
  * seq_get_buf - get buffer to write arbitrary data to
  * @m: the seq_file handle
  * @bufp: the beginning of the buffer is stored here
-- 
2.1.1

Re: [RFA][PATCH 1/8] seq_file: Rename seq_overflow() to seq_has_overflowed() and make public

[Joe Perches]

On Wed, 2014-10-29 at 17:56 -0400, Steven Rostedt wrote:
> +A true return from seq_has_overflowed means that the seq_file buffer is full
> +and further output will be discarded.

Perhaps the description is a bit unclear here.

Doesn't a return of true to seq_has_overflowed mean that
more characters have already been written than the buffer
can accept?

Re: [RFA][PATCH 1/8] seq_file: Rename seq_overflow() to seq_has_overflowed() and make public

[Steven Rostedt]

On Wed, 29 Oct 2014 15:08:36 -0700
Joe Perches <joe@perches.com> wrote:

> On Wed, 2014-10-29 at 17:56 -0400, Steven Rostedt wrote:
> > +A true return from seq_has_overflowed means that the seq_file buffer is full
> > +and further output will be discarded.
> 
> Perhaps the description is a bit unclear here.
> 
> Doesn't a return of true to seq_has_overflowed mean that
> more characters have already been written than the buffer
> can accept?
> 

Actually, right now the comment is correct and the name is misleading.
But I have a patch that will make the comment incorrect (and will be
fixed) and the name correct.

But since seq_has_overflowed() is to be used throughout the kernel, I
didn't want to have to go do patches all over again for a temporary
misnomer.

In other words, I'm going to change the function to really return only
if it did overflow and not just be full. But that's another patch set
to come.

I can update the change log and comment to state what will happen in
the future.

Thanks,

-- Steve

Re: [RFA][PATCH 1/8] seq_file: Rename seq_overflow() to seq_has_overflowed() and make public

[Joe Perches]

On Wed, 2014-10-29 at 19:42 -0400, Steven Rostedt wrote:
> On Wed, 29 Oct 2014 15:08:36 -0700 Joe Perches <joe@perches.com> wrote:
> > On Wed, 2014-10-29 at 17:56 -0400, Steven Rostedt wrote:
> > > +A true return from seq_has_overflowed means that the seq_file buffer is full
> > > +and further output will be discarded.
> > Perhaps the description is a bit unclear here.
> > Doesn't a return of true to seq_has_overflowed mean that
> > more characters have already been written than the buffer
> > can accept?
> Actually, right now the comment is correct and the name is misleading.
> But I have a patch that will make the comment incorrect (and will be
> fixed) and the name correct.
> 
> But since seq_has_overflowed() is to be used throughout the kernel, I
> didn't want to have to go do patches all over again for a temporary
> misnomer.

I think it'd be better if the first submission
of the function has the correct operation for
the name.

> In other words, I'm going to change the function to really return only
> if it did overflow and not just be full. But that's another patch set
> to come.

Why would it be a set and not a single patch?

Re: [RFA][PATCH 1/8] seq_file: Rename seq_overflow() to seq_has_overflowed() and make public

[Steven Rostedt]

On Wed, 29 Oct 2014 16:53:30 -0700
Joe Perches <joe@perches.com> wrote:

> On Wed, 2014-10-29 at 19:42 -0400, Steven Rostedt wrote:
> > On Wed, 29 Oct 2014 15:08:36 -0700 Joe Perches <joe@perches.com> wrote:
> > > On Wed, 2014-10-29 at 17:56 -0400, Steven Rostedt wrote:
> > > > +A true return from seq_has_overflowed means that the seq_file buffer is full
> > > > +and further output will be discarded.
> > > Perhaps the description is a bit unclear here.
> > > Doesn't a return of true to seq_has_overflowed mean that
> > > more characters have already been written than the buffer
> > > can accept?
> > Actually, right now the comment is correct and the name is misleading.
> > But I have a patch that will make the comment incorrect (and will be
> > fixed) and the name correct.
> > 
> > But since seq_has_overflowed() is to be used throughout the kernel, I
> > didn't want to have to go do patches all over again for a temporary
> > misnomer.
> 
> I think it'd be better if the first submission
> of the function has the correct operation for
> the name.

I'm hoping that both of these will make it into 3.19, so it really
doesn't matter the order. I'm working on this patch first, because, as
you said earlier, it is important to get the call to
seq_has_overflowed() out there.

> 
> > In other words, I'm going to change the function to really return only
> > if it did overflow and not just be full. But that's another patch set
> > to come.
> 
> Why would it be a set and not a single patch?
> 

That particular change is a single patch. But it is in with other
patches I'm doing, so I called it a patch set.

-- Steve

Re: [RFA][PATCH 1/8] seq_file: Rename seq_overflow() to seq_has_overflowed() and make public

[Joe Perches]

On Wed, 2014-10-29 at 20:10 -0400, Steven Rostedt wrote:
> On Wed, 29 Oct 2014 16:53:30 -0700
> Joe Perches <joe@perches.com> wrote:
> 
> > On Wed, 2014-10-29 at 19:42 -0400, Steven Rostedt wrote:
> > > On Wed, 29 Oct 2014 15:08:36 -0700 Joe Perches <joe@perches.com> wrote:
> > > > On Wed, 2014-10-29 at 17:56 -0400, Steven Rostedt wrote:
> > > > > +A true return from seq_has_overflowed means that the seq_file buffer is full
> > > > > +and further output will be discarded.
> > > > Perhaps the description is a bit unclear here.
> > > > Doesn't a return of true to seq_has_overflowed mean that
> > > > more characters have already been written than the buffer
> > > > can accept?
> > > Actually, right now the comment is correct and the name is misleading.
> > > But I have a patch that will make the comment incorrect (and will be
> > > fixed) and the name correct.
> > > 
> > > But since seq_has_overflowed() is to be used throughout the kernel, I
> > > didn't want to have to go do patches all over again for a temporary
> > > misnomer.
> > 
> > I think it'd be better if the first submission
> > of the function has the correct operation for
> > the name.
> 
> I'm hoping that both of these will make it into 3.19, so it really
> doesn't matter the order. I'm working on this patch first, because, as
> you said earlier, it is important to get the call to
> seq_has_overflowed() out there.

So, given the necessary sequence of adding this new
function before any other change is made, please make
this function correct at the get-go.

Re: [RFA][PATCH 1/8] seq_file: Rename seq_overflow() to seq_has_overflowed() and make public

[Steven Rostedt]

On Wed, 29 Oct 2014 15:08:36 -0700
Joe Perches <joe@perches.com> wrote:

> On Wed, 2014-10-29 at 17:56 -0400, Steven Rostedt wrote:
> > +A true return from seq_has_overflowed means that the seq_file buffer is full
> > +and further output will be discarded.
> 
> Perhaps the description is a bit unclear here.
> 
> Doesn't a return of true to seq_has_overflowed mean that
> more characters have already been written than the buffer
> can accept?
> 

Actually, come to think about it. It really doesn't matter. As if it is
"full" or overflowed, the buffer currently gets discarded regardless.
Thus, it could mean that the buffer was already overflowed. Because the
current logic is full == overflowed. Really, a full buffer is a buffer
with all but one byte left used. If size == len, then it has indeed
overflowed.

That is what my other patch changes.

I'll change the comment about that.

-- Steve

Re: [RFA][PATCH 1/8] seq_file: Rename seq_overflow() to seq_has_overflowed() and make public

[Steven Rostedt]

On Wed, 29 Oct 2014 20:20:45 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:


> I'll change the comment about that.

How's this?

-- Steve


The return values of seq_printf/puts/putc are frequently misused.

Start down a path to remove all the return value uses of these
functions.

Move the seq_overflow() to a global inlined function called
seq_has_overflowed() that can be used by the users of seq_file() calls.

Update the documentation to not show return types for seq_printf
et al.  Add a description of seq_has_overflowed().

Link: http://lkml.kernel.org/p/848ac7e3d1c31cddf638a8526fa3c59fa6fdeb8a.1412031505.git.joe@perches.com

Cc: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Joe Perches <joe@perches.com>
[ Reworked the original patch from Joe ]
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 Documentation/filesystems/seq_file.txt | 22 +++++++++++++---------
 fs/seq_file.c                          | 15 ++-------------
 include/linux/seq_file.h               | 15 +++++++++++++++
 3 files changed, 30 insertions(+), 22 deletions(-)

diff --git a/Documentation/filesystems/seq_file.txt b/Documentation/filesystems/seq_file.txt
index 8ea3e90ace07..b797ed38de46 100644
--- a/Documentation/filesystems/seq_file.txt
+++ b/Documentation/filesystems/seq_file.txt
@@ -180,23 +180,19 @@ output must be passed to the seq_file code. Some utility functions have
 been defined which make this task easy.
 
 Most code will simply use seq_printf(), which works pretty much like
-printk(), but which requires the seq_file pointer as an argument. It is
-common to ignore the return value from seq_printf(), but a function
-producing complicated output may want to check that value and quit if
-something non-zero is returned; an error return means that the seq_file
-buffer has been filled and further output will be discarded.
+printk(), but which requires the seq_file pointer as an argument.
 
 For straight character output, the following functions may be used:
 
-	int seq_putc(struct seq_file *m, char c);
-	int seq_puts(struct seq_file *m, const char *s);
-	int seq_escape(struct seq_file *m, const char *s, const char *esc);
+	seq_putc(struct seq_file *m, char c);
+	seq_puts(struct seq_file *m, const char *s);
+	seq_escape(struct seq_file *m, const char *s, const char *esc);
 
 The first two output a single character and a string, just like one would
 expect. seq_escape() is like seq_puts(), except that any character in s
 which is in the string esc will be represented in octal form in the output.
 
-There is also a pair of functions for printing filenames:
+There are also a pair of functions for printing filenames:
 
 	int seq_path(struct seq_file *m, struct path *path, char *esc);
 	int seq_path_root(struct seq_file *m, struct path *path,
@@ -209,6 +205,14 @@ root is desired, it can be used with seq_path_root().  Note that, if it
 turns out that path cannot be reached from root, the value of root will be
 changed in seq_file_root() to a root which *does* work.
 
+A function producing complicated output may want to check
+	bool seq_has_overflowed(struct seq_file *m);
+and avoid further seq_<output> calls if true is returned.
+
+A true return from seq_has_overflowed means that the seq_file buffer will
+be discarded and the seq_show function will attempt to allocate a larger
+buffer and retry printing.
+
 
 Making it all work
 
diff --git a/fs/seq_file.c b/fs/seq_file.c
index 3857b720cb1b..353948ba1c5b 100644
--- a/fs/seq_file.c
+++ b/fs/seq_file.c
@@ -16,17 +16,6 @@
 #include <asm/uaccess.h>
 #include <asm/page.h>
 
-
-/*
- * seq_files have a buffer which can may overflow. When this happens a larger
- * buffer is reallocated and all the data will be printed again.
- * The overflow state is true when m->count == m->size.
- */
-static bool seq_overflow(struct seq_file *m)
-{
-	return m->count == m->size;
-}
-
 static void seq_set_overflow(struct seq_file *m)
 {
 	m->count = m->size;
@@ -124,7 +113,7 @@ static int traverse(struct seq_file *m, loff_t offset)
 			error = 0;
 			m->count = 0;
 		}
-		if (seq_overflow(m))
+		if (seq_has_overflowed(m))
 			goto Eoverflow;
 		if (pos + m->count > offset) {
 			m->from = offset - pos;
@@ -267,7 +256,7 @@ Fill:
 			break;
 		}
 		err = m->op->show(m, p);
-		if (seq_overflow(m) || err) {
+		if (seq_has_overflowed(m) || err) {
 			m->count = offs;
 			if (likely(err <= 0))
 				break;
diff --git a/include/linux/seq_file.h b/include/linux/seq_file.h
index 52e0097f61f0..cf6a9daaaf6d 100644
--- a/include/linux/seq_file.h
+++ b/include/linux/seq_file.h
@@ -43,6 +43,21 @@ struct seq_operations {
 #define SEQ_SKIP 1
 
 /**
+ * seq_has_overflowed - check if the buffer has overflowed
+ * @m: the seq_file handle
+ *
+ * seq_files have a buffer which may overflow. When this happens a larger
+ * buffer is reallocated and all the data will be printed again.
+ * The overflow state is true when m->count == m->size.
+ *
+ * Returns true if the buffer received more than it can hold.
+ */
+static inline bool seq_has_overflowed(struct seq_file *m)
+{
+	return m->count == m->size;
+}
+
+/**
  * seq_get_buf - get buffer to write arbitrary data to
  * @m: the seq_file handle
  * @bufp: the beginning of the buffer is stored here
-- 
1.8.1.4

Re: [RFA][PATCH 1/8] seq_file: Rename seq_overflow() to seq_has_overflowed() and make public

[Joe Perches]

On Wed, 2014-10-29 at 20:27 -0400, Steven Rostedt wrote:
> On Wed, 29 Oct 2014 20:20:45 -0400
> Steven Rostedt <rostedt@goodmis.org> wrote:
> > I'll change the comment about that.
> 
> How's this?

Fine, but I think whether a function is inlined or
not is largely irrelevant to functionality.

cheers, Joe

> Move the seq_overflow() to a global inlined function called
> seq_has_overflowed() that can be used by the users of
> seq_file() calls.

[RFA][PATCH 2/8] netfilter: Remove return values for print_conntrack callbacks

[Steven Rostedt]

[-- Attachment #1: 0002-netfilter-Remove-return-values-for-print_conntrack-c.patch --]
[-- Type: text/plain, Size: 6249 bytes --]

From: "Steven Rostedt (Red Hat)" <rostedt@goodmis.org>

[ REQUEST FOR ACKS ]

The seq_printf() and friends are having their return values removed.
The print_conntrack() returns the result of seq_printf(), which is
meaningless when seq_printf() returns void. Might as well remove the
return values of print_conntrack() as well.

Cc: Pablo Neira Ayuso <pablo@netfilter.org>
Cc: Patrick McHardy <kaber@trash.net>
Cc: Jozsef Kadlecsik <kadlec@blackhole.kfki.hu>
Cc: netfilter-devel@vger.kernel.org
Cc: coreteam@netfilter.org
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 include/net/netfilter/nf_conntrack_l4proto.h          | 2 +-
 net/ipv4/netfilter/nf_conntrack_l3proto_ipv4_compat.c | 5 ++++-
 net/netfilter/nf_conntrack_proto_dccp.c               | 4 ++--
 net/netfilter/nf_conntrack_proto_gre.c                | 8 ++++----
 net/netfilter/nf_conntrack_proto_sctp.c               | 4 ++--
 net/netfilter/nf_conntrack_proto_tcp.c                | 4 ++--
 net/netfilter/nf_conntrack_standalone.c               | 4 ++--
 7 files changed, 17 insertions(+), 14 deletions(-)

diff --git a/include/net/netfilter/nf_conntrack_l4proto.h b/include/net/netfilter/nf_conntrack_l4proto.h
index 4c8d573830b7..82e4ec002a39 100644
--- a/include/net/netfilter/nf_conntrack_l4proto.h
+++ b/include/net/netfilter/nf_conntrack_l4proto.h
@@ -60,7 +60,7 @@ struct nf_conntrack_l4proto {
 			   const struct nf_conntrack_tuple *);
 
 	/* Print out the private part of the conntrack. */
-	int (*print_conntrack)(struct seq_file *s, struct nf_conn *);
+	void (*print_conntrack)(struct seq_file *s, struct nf_conn *);
 
 	/* Return the array of timeouts for this protocol. */
 	unsigned int *(*get_timeouts)(struct net *net);
diff --git a/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4_compat.c b/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4_compat.c
index 4c48e434bb1f..91f207c2cb69 100644
--- a/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4_compat.c
+++ b/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4_compat.c
@@ -147,7 +147,10 @@ static int ct_seq_show(struct seq_file *s, void *v)
 		      ? (long)(ct->timeout.expires - jiffies)/HZ : 0) != 0)
 		goto release;
 
-	if (l4proto->print_conntrack && l4proto->print_conntrack(s, ct))
+	if (l4proto->print_conntrack)
+		l4proto->print_conntrack(s, ct);
+
+	if (seq_has_overflowed(s))
 		goto release;
 
 	if (print_tuple(s, &ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple,
diff --git a/net/netfilter/nf_conntrack_proto_dccp.c b/net/netfilter/nf_conntrack_proto_dccp.c
index cb372f96f10d..15971177470a 100644
--- a/net/netfilter/nf_conntrack_proto_dccp.c
+++ b/net/netfilter/nf_conntrack_proto_dccp.c
@@ -626,9 +626,9 @@ static int dccp_print_tuple(struct seq_file *s,
 			  ntohs(tuple->dst.u.dccp.port));
 }
 
-static int dccp_print_conntrack(struct seq_file *s, struct nf_conn *ct)
+static void dccp_print_conntrack(struct seq_file *s, struct nf_conn *ct)
 {
-	return seq_printf(s, "%s ", dccp_state_names[ct->proto.dccp.state]);
+	seq_printf(s, "%s ", dccp_state_names[ct->proto.dccp.state]);
 }
 
 #if IS_ENABLED(CONFIG_NF_CT_NETLINK)
diff --git a/net/netfilter/nf_conntrack_proto_gre.c b/net/netfilter/nf_conntrack_proto_gre.c
index d5665739e3b1..cba607ada069 100644
--- a/net/netfilter/nf_conntrack_proto_gre.c
+++ b/net/netfilter/nf_conntrack_proto_gre.c
@@ -235,11 +235,11 @@ static int gre_print_tuple(struct seq_file *s,
 }
 
 /* print private data for conntrack */
-static int gre_print_conntrack(struct seq_file *s, struct nf_conn *ct)
+static void gre_print_conntrack(struct seq_file *s, struct nf_conn *ct)
 {
-	return seq_printf(s, "timeout=%u, stream_timeout=%u ",
-			  (ct->proto.gre.timeout / HZ),
-			  (ct->proto.gre.stream_timeout / HZ));
+	seq_printf(s, "timeout=%u, stream_timeout=%u ",
+		   (ct->proto.gre.timeout / HZ),
+		   (ct->proto.gre.stream_timeout / HZ));
 }
 
 static unsigned int *gre_get_timeouts(struct net *net)
diff --git a/net/netfilter/nf_conntrack_proto_sctp.c b/net/netfilter/nf_conntrack_proto_sctp.c
index 1314d33f6bcf..c61f4cd6407d 100644
--- a/net/netfilter/nf_conntrack_proto_sctp.c
+++ b/net/netfilter/nf_conntrack_proto_sctp.c
@@ -175,7 +175,7 @@ static int sctp_print_tuple(struct seq_file *s,
 }
 
 /* Print out the private part of the conntrack. */
-static int sctp_print_conntrack(struct seq_file *s, struct nf_conn *ct)
+static void sctp_print_conntrack(struct seq_file *s, struct nf_conn *ct)
 {
 	enum sctp_conntrack state;
 
@@ -183,7 +183,7 @@ static int sctp_print_conntrack(struct seq_file *s, struct nf_conn *ct)
 	state = ct->proto.sctp.state;
 	spin_unlock_bh(&ct->lock);
 
-	return seq_printf(s, "%s ", sctp_conntrack_names[state]);
+	seq_printf(s, "%s ", sctp_conntrack_names[state]);
 }
 
 #define for_each_sctp_chunk(skb, sch, _sch, offset, dataoff, count)	\
diff --git a/net/netfilter/nf_conntrack_proto_tcp.c b/net/netfilter/nf_conntrack_proto_tcp.c
index 44d1ea32570a..79668fd3db96 100644
--- a/net/netfilter/nf_conntrack_proto_tcp.c
+++ b/net/netfilter/nf_conntrack_proto_tcp.c
@@ -311,7 +311,7 @@ static int tcp_print_tuple(struct seq_file *s,
 }
 
 /* Print out the private part of the conntrack. */
-static int tcp_print_conntrack(struct seq_file *s, struct nf_conn *ct)
+static void tcp_print_conntrack(struct seq_file *s, struct nf_conn *ct)
 {
 	enum tcp_conntrack state;
 
@@ -319,7 +319,7 @@ static int tcp_print_conntrack(struct seq_file *s, struct nf_conn *ct)
 	state = ct->proto.tcp.state;
 	spin_unlock_bh(&ct->lock);
 
-	return seq_printf(s, "%s ", tcp_conntrack_names[state]);
+	seq_printf(s, "%s ", tcp_conntrack_names[state]);
 }
 
 static unsigned int get_conntrack_index(const struct tcphdr *tcph)
diff --git a/net/netfilter/nf_conntrack_standalone.c b/net/netfilter/nf_conntrack_standalone.c
index cf65a1e040dd..348aa3602787 100644
--- a/net/netfilter/nf_conntrack_standalone.c
+++ b/net/netfilter/nf_conntrack_standalone.c
@@ -199,8 +199,8 @@ static int ct_seq_show(struct seq_file *s, void *v)
 		       ? (long)(ct->timeout.expires - jiffies)/HZ : 0) != 0)
 		goto release;
 
-	if (l4proto->print_conntrack && l4proto->print_conntrack(s, ct))
-		goto release;
+	if (l4proto->print_conntrack)
+		l4proto->print_conntrack(s, ct);
 
 	if (print_tuple(s, &ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple,
 			l3proto, l4proto))
-- 
2.1.1

Re: [RFA][PATCH 2/8] netfilter: Remove return values for print_conntrack callbacks

[Florian Westphal]

Steven Rostedt <rostedt@goodmis.org> wrote:
> From: "Steven Rostedt (Red Hat)" <rostedt@goodmis.org>
> 
> [ REQUEST FOR ACKS ]
> 
> The seq_printf() and friends are having their return values removed.
> The print_conntrack() returns the result of seq_printf(), which is
> meaningless when seq_printf() returns void. Might as well remove the
> return values of print_conntrack() as well.
[..]

> diff --git a/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4_compat.c b/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4_compat.c
> index 4c48e434bb1f..91f207c2cb69 100644
> --- a/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4_compat.c
> +++ b/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4_compat.c
> @@ -147,7 +147,10 @@ static int ct_seq_show(struct seq_file *s, void *v)
>  		      ? (long)(ct->timeout.expires - jiffies)/HZ : 0) != 0)
>  		goto release;
>  
> -	if (l4proto->print_conntrack && l4proto->print_conntrack(s, ct))
> +	if (l4proto->print_conntrack)
> +		l4proto->print_conntrack(s, ct);
> +
> +	if (seq_has_overflowed(s))
>  		goto release;

Its not obvious to me why nf_conntrack_l3proto_ipv4_compat now calls
seq_has_overflowed ...

> > diff --git a/net/netfilter/nf_conntrack_standalone.c b/net/netfilter/nf_conntrack_standalone.c
> index cf65a1e040dd..348aa3602787 100644
> --- a/net/netfilter/nf_conntrack_standalone.c
> +++ b/net/netfilter/nf_conntrack_standalone.c
> @@ -199,8 +199,8 @@ static int ct_seq_show(struct seq_file *s, void *v)
>  		       ? (long)(ct->timeout.expires - jiffies)/HZ : 0) != 0)
>  		goto release;
>  
> -	if (l4proto->print_conntrack && l4proto->print_conntrack(s, ct))
> -		goto release;
> +	if (l4proto->print_conntrack)
> +		l4proto->print_conntrack(s, ct);
>  
>  	if (print_tuple(s, &ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple,
>  			l3proto, l4proto))

... while nf_conntrack_standalone does not.

Re: [RFA][PATCH 2/8] netfilter: Remove return values for print_conntrack callbacks

[Steven Rostedt]

On Wed, 29 Oct 2014 23:16:01 +0100
Florian Westphal <fw@strlen.de> wrote:

> Steven Rostedt <rostedt@goodmis.org> wrote:
> > From: "Steven Rostedt (Red Hat)" <rostedt@goodmis.org>
> > 
> > [ REQUEST FOR ACKS ]
> > 
> > The seq_printf() and friends are having their return values removed.
> > The print_conntrack() returns the result of seq_printf(), which is
> > meaningless when seq_printf() returns void. Might as well remove the
> > return values of print_conntrack() as well.
> [..]
> 
> > diff --git a/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4_compat.c b/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4_compat.c
> > index 4c48e434bb1f..91f207c2cb69 100644
> > --- a/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4_compat.c
> > +++ b/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4_compat.c
> > @@ -147,7 +147,10 @@ static int ct_seq_show(struct seq_file *s, void *v)
> >  		      ? (long)(ct->timeout.expires - jiffies)/HZ : 0) != 0)
> >  		goto release;
> >  
> > -	if (l4proto->print_conntrack && l4proto->print_conntrack(s, ct))
> > +	if (l4proto->print_conntrack)
> > +		l4proto->print_conntrack(s, ct);
> > +
> > +	if (seq_has_overflowed(s))
> >  		goto release;
> 
> Its not obvious to me why nf_conntrack_l3proto_ipv4_compat now calls
> seq_has_overflowed ...
> 
> > > diff --git a/net/netfilter/nf_conntrack_standalone.c b/net/netfilter/nf_conntrack_standalone.c
> > index cf65a1e040dd..348aa3602787 100644
> > --- a/net/netfilter/nf_conntrack_standalone.c
> > +++ b/net/netfilter/nf_conntrack_standalone.c
> > @@ -199,8 +199,8 @@ static int ct_seq_show(struct seq_file *s, void *v)
> >  		       ? (long)(ct->timeout.expires - jiffies)/HZ : 0) != 0)
> >  		goto release;
> >  
> > -	if (l4proto->print_conntrack && l4proto->print_conntrack(s, ct))
> > -		goto release;
> > +	if (l4proto->print_conntrack)
> > +		l4proto->print_conntrack(s, ct);
> >  
> >  	if (print_tuple(s, &ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple,
> >  			l3proto, l4proto))
> 
> ... while nf_conntrack_standalone does not.

Because I did those two changes separately at different times ;-)

I can add it here too to be consistent. The goto release logic is not
that critical, as it's really a micro optimization for a slow path.

If we were to remove all the goto release calls, what would happen is
if the buffer were to fill up, the rest of the code would be done in
vain, because it would do the work but the result will be thrown away.
The seq_file code would throw away the buffer completely (does this
regardless of the goto release call when the buffer fills up), and
would allocate a new buffer, and then run the code again. This time it
would hopefully have enough buffer space to hold the entire content,
otherwise, wash rinse repeat.

As this is to output text from a proc file, and the user will most
likely never notice this delay. It's probably done as more of a good
feeling that we didn't waste computer cycles and burn more green house
gasses than necessary, then anything that is remotely noticeable.
Although, I haven't run benchmarks to see what the difference is.

I'll make the change for consistency.

Thanks!

-- Steve

Re: [RFA][PATCH 2/8] netfilter: Remove return values for print_conntrack callbacks

[Steven Rostedt]

On Wed, 29 Oct 2014 23:16:01 +0100
Florian Westphal <fw@strlen.de> wrote:

> Steven Rostedt <rostedt@goodmis.org> wrote:
> > From: "Steven Rostedt (Red Hat)" <rostedt@goodmis.org>
> > 
> > [ REQUEST FOR ACKS ]
> > 
> > The seq_printf() and friends are having their return values removed.
> > The print_conntrack() returns the result of seq_printf(), which is
> > meaningless when seq_printf() returns void. Might as well remove the
> > return values of print_conntrack() as well.
> [..]
> 
> > diff --git a/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4_compat.c b/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4_compat.c
> > index 4c48e434bb1f..91f207c2cb69 100644
> > --- a/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4_compat.c
> > +++ b/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4_compat.c
> > @@ -147,7 +147,10 @@ static int ct_seq_show(struct seq_file *s, void *v)
> >  		      ? (long)(ct->timeout.expires - jiffies)/HZ : 0) != 0)
> >  		goto release;
> >  
> > -	if (l4proto->print_conntrack && l4proto->print_conntrack(s, ct))
> > +	if (l4proto->print_conntrack)
> > +		l4proto->print_conntrack(s, ct);
> > +
> > +	if (seq_has_overflowed(s))
> >  		goto release;
> 
> Its not obvious to me why nf_conntrack_l3proto_ipv4_compat now calls
> seq_has_overflowed ...
> 
> > > diff --git a/net/netfilter/nf_conntrack_standalone.c b/net/netfilter/nf_conntrack_standalone.c
> > index cf65a1e040dd..348aa3602787 100644
> > --- a/net/netfilter/nf_conntrack_standalone.c
> > +++ b/net/netfilter/nf_conntrack_standalone.c
> > @@ -199,8 +199,8 @@ static int ct_seq_show(struct seq_file *s, void *v)
> >  		       ? (long)(ct->timeout.expires - jiffies)/HZ : 0) != 0)
> >  		goto release;
> >  
> > -	if (l4proto->print_conntrack && l4proto->print_conntrack(s, ct))
> > -		goto release;
> > +	if (l4proto->print_conntrack)
> > +		l4proto->print_conntrack(s, ct);
> >  
> >  	if (print_tuple(s, &ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple,
> >  			l3proto, l4proto))
> 
> ... while nf_conntrack_standalone does not.

Oh, looks like it was added here in patch 3/8.

The reason for the two patches is that patch 3/8 is from Joe. I added
places that he missed, and put that patch before his (2/8).

-- Steve

Re: [RFA][PATCH 2/8] netfilter: Remove return values for print_conntrack callbacks

[Steven Rostedt]

On Wed, 29 Oct 2014 17:56:04 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

> From: "Steven Rostedt (Red Hat)" <rostedt@goodmis.org>
> 
> [ REQUEST FOR ACKS ]

Can any of the netfilter folks give me an Acked-by for this?

Thanks!

-- Steve

> 
> The seq_printf() and friends are having their return values removed.
> The print_conntrack() returns the result of seq_printf(), which is
> meaningless when seq_printf() returns void. Might as well remove the
> return values of print_conntrack() as well.
> 
> Cc: Pablo Neira Ayuso <pablo@netfilter.org>
> Cc: Patrick McHardy <kaber@trash.net>
> Cc: Jozsef Kadlecsik <kadlec@blackhole.kfki.hu>
> Cc: netfilter-devel@vger.kernel.org
> Cc: coreteam@netfilter.org
> Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
> ---
>  include/net/netfilter/nf_conntrack_l4proto.h          | 2 +-
>  net/ipv4/netfilter/nf_conntrack_l3proto_ipv4_compat.c | 5 ++++-
>  net/netfilter/nf_conntrack_proto_dccp.c               | 4 ++--
>  net/netfilter/nf_conntrack_proto_gre.c                | 8 ++++----
>  net/netfilter/nf_conntrack_proto_sctp.c               | 4 ++--
>  net/netfilter/nf_conntrack_proto_tcp.c                | 4 ++--
>  net/netfilter/nf_conntrack_standalone.c               | 4 ++--
>  7 files changed, 17 insertions(+), 14 deletions(-)
> 
> diff --git a/include/net/netfilter/nf_conntrack_l4proto.h b/include/net/netfilter/nf_conntrack_l4proto.h
> index 4c8d573830b7..82e4ec002a39 100644
> --- a/include/net/netfilter/nf_conntrack_l4proto.h
> +++ b/include/net/netfilter/nf_conntrack_l4proto.h
> @@ -60,7 +60,7 @@ struct nf_conntrack_l4proto {
>  			   const struct nf_conntrack_tuple *);
>  
>  	/* Print out the private part of the conntrack. */
> -	int (*print_conntrack)(struct seq_file *s, struct nf_conn *);
> +	void (*print_conntrack)(struct seq_file *s, struct nf_conn *);
>  
>  	/* Return the array of timeouts for this protocol. */
>  	unsigned int *(*get_timeouts)(struct net *net);
> diff --git a/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4_compat.c b/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4_compat.c
> index 4c48e434bb1f..91f207c2cb69 100644
> --- a/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4_compat.c
> +++ b/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4_compat.c
> @@ -147,7 +147,10 @@ static int ct_seq_show(struct seq_file *s, void *v)
>  		      ? (long)(ct->timeout.expires - jiffies)/HZ : 0) != 0)
>  		goto release;
>  
> -	if (l4proto->print_conntrack && l4proto->print_conntrack(s, ct))
> +	if (l4proto->print_conntrack)
> +		l4proto->print_conntrack(s, ct);
> +
> +	if (seq_has_overflowed(s))
>  		goto release;
>  
>  	if (print_tuple(s, &ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple,
> diff --git a/net/netfilter/nf_conntrack_proto_dccp.c b/net/netfilter/nf_conntrack_proto_dccp.c
> index cb372f96f10d..15971177470a 100644
> --- a/net/netfilter/nf_conntrack_proto_dccp.c
> +++ b/net/netfilter/nf_conntrack_proto_dccp.c
> @@ -626,9 +626,9 @@ static int dccp_print_tuple(struct seq_file *s,
>  			  ntohs(tuple->dst.u.dccp.port));
>  }
>  
> -static int dccp_print_conntrack(struct seq_file *s, struct nf_conn *ct)
> +static void dccp_print_conntrack(struct seq_file *s, struct nf_conn *ct)
>  {
> -	return seq_printf(s, "%s ", dccp_state_names[ct->proto.dccp.state]);
> +	seq_printf(s, "%s ", dccp_state_names[ct->proto.dccp.state]);
>  }
>  
>  #if IS_ENABLED(CONFIG_NF_CT_NETLINK)
> diff --git a/net/netfilter/nf_conntrack_proto_gre.c b/net/netfilter/nf_conntrack_proto_gre.c
> index d5665739e3b1..cba607ada069 100644
> --- a/net/netfilter/nf_conntrack_proto_gre.c
> +++ b/net/netfilter/nf_conntrack_proto_gre.c
> @@ -235,11 +235,11 @@ static int gre_print_tuple(struct seq_file *s,
>  }
>  
>  /* print private data for conntrack */
> -static int gre_print_conntrack(struct seq_file *s, struct nf_conn *ct)
> +static void gre_print_conntrack(struct seq_file *s, struct nf_conn *ct)
>  {
> -	return seq_printf(s, "timeout=%u, stream_timeout=%u ",
> -			  (ct->proto.gre.timeout / HZ),
> -			  (ct->proto.gre.stream_timeout / HZ));
> +	seq_printf(s, "timeout=%u, stream_timeout=%u ",
> +		   (ct->proto.gre.timeout / HZ),
> +		   (ct->proto.gre.stream_timeout / HZ));
>  }
>  
>  static unsigned int *gre_get_timeouts(struct net *net)
> diff --git a/net/netfilter/nf_conntrack_proto_sctp.c b/net/netfilter/nf_conntrack_proto_sctp.c
> index 1314d33f6bcf..c61f4cd6407d 100644
> --- a/net/netfilter/nf_conntrack_proto_sctp.c
> +++ b/net/netfilter/nf_conntrack_proto_sctp.c
> @@ -175,7 +175,7 @@ static int sctp_print_tuple(struct seq_file *s,
>  }
>  
>  /* Print out the private part of the conntrack. */
> -static int sctp_print_conntrack(struct seq_file *s, struct nf_conn *ct)
> +static void sctp_print_conntrack(struct seq_file *s, struct nf_conn *ct)
>  {
>  	enum sctp_conntrack state;
>  
> @@ -183,7 +183,7 @@ static int sctp_print_conntrack(struct seq_file *s, struct nf_conn *ct)
>  	state = ct->proto.sctp.state;
>  	spin_unlock_bh(&ct->lock);
>  
> -	return seq_printf(s, "%s ", sctp_conntrack_names[state]);
> +	seq_printf(s, "%s ", sctp_conntrack_names[state]);
>  }
>  
>  #define for_each_sctp_chunk(skb, sch, _sch, offset, dataoff, count)	\
> diff --git a/net/netfilter/nf_conntrack_proto_tcp.c b/net/netfilter/nf_conntrack_proto_tcp.c
> index 44d1ea32570a..79668fd3db96 100644
> --- a/net/netfilter/nf_conntrack_proto_tcp.c
> +++ b/net/netfilter/nf_conntrack_proto_tcp.c
> @@ -311,7 +311,7 @@ static int tcp_print_tuple(struct seq_file *s,
>  }
>  
>  /* Print out the private part of the conntrack. */
> -static int tcp_print_conntrack(struct seq_file *s, struct nf_conn *ct)
> +static void tcp_print_conntrack(struct seq_file *s, struct nf_conn *ct)
>  {
>  	enum tcp_conntrack state;
>  
> @@ -319,7 +319,7 @@ static int tcp_print_conntrack(struct seq_file *s, struct nf_conn *ct)
>  	state = ct->proto.tcp.state;
>  	spin_unlock_bh(&ct->lock);
>  
> -	return seq_printf(s, "%s ", tcp_conntrack_names[state]);
> +	seq_printf(s, "%s ", tcp_conntrack_names[state]);
>  }
>  
>  static unsigned int get_conntrack_index(const struct tcphdr *tcph)
> diff --git a/net/netfilter/nf_conntrack_standalone.c b/net/netfilter/nf_conntrack_standalone.c
> index cf65a1e040dd..348aa3602787 100644
> --- a/net/netfilter/nf_conntrack_standalone.c
> +++ b/net/netfilter/nf_conntrack_standalone.c
> @@ -199,8 +199,8 @@ static int ct_seq_show(struct seq_file *s, void *v)
>  		       ? (long)(ct->timeout.expires - jiffies)/HZ : 0) != 0)
>  		goto release;
>  
> -	if (l4proto->print_conntrack && l4proto->print_conntrack(s, ct))
> -		goto release;
> +	if (l4proto->print_conntrack)
> +		l4proto->print_conntrack(s, ct);
>  
>  	if (print_tuple(s, &ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple,
>  			l3proto, l4proto))

Re: [RFA][PATCH 2/8] netfilter: Remove return values for print_conntrack callbacks

[Steven Rostedt]

On Tue, 4 Nov 2014 08:05:35 -0500
Steven Rostedt <rostedt@goodmis.org> wrote:

> On Wed, 29 Oct 2014 17:56:04 -0400
> Steven Rostedt <rostedt@goodmis.org> wrote:
> 
> > From: "Steven Rostedt (Red Hat)" <rostedt@goodmis.org>
> > 
> > [ REQUEST FOR ACKS ]
> 
> Can any of the netfilter folks give me an Acked-by for this?
> 

I hope nobody expects Jozsef Kadlecsik to do this, as I'm getting "Mail
Delivery Failures" from him with this message:

>>> kadlec@blackhole.kfki.hu (after RCPT TO): 550 5.7.1 <kadlec@blackhole.kfki.hu>: Recipient address rejected: Access denied. Your site is banned because of the unsolicited mail messages received from it.  

Wow, seems they are banning all Time Warner Cable customers in my area!

-- Steve

Re: [RFA][PATCH 2/8] netfilter: Remove return values for print_conntrack callbacks

[Pablo Neira Ayuso]

On Tue, Nov 04, 2014 at 08:05:35AM -0500, Steven Rostedt wrote:
> On Wed, 29 Oct 2014 17:56:04 -0400
> Steven Rostedt <rostedt@goodmis.org> wrote:
> 
> > From: "Steven Rostedt (Red Hat)" <rostedt@goodmis.org>
> > 
> > [ REQUEST FOR ACKS ]
> 
> Can any of the netfilter folks give me an Acked-by for this?

If Florian's concern were addressed, then:

Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>

to this patch and 4/8.

Re: [RFA][PATCH 2/8] netfilter: Remove return values for print_conntrack callbacks

[Steven Rostedt]

On Tue, 4 Nov 2014 15:22:36 +0100
Pablo Neira Ayuso <pablo@netfilter.org> wrote:

> On Tue, Nov 04, 2014 at 08:05:35AM -0500, Steven Rostedt wrote:
> > On Wed, 29 Oct 2014 17:56:04 -0400
> > Steven Rostedt <rostedt@goodmis.org> wrote:
> > 
> > > From: "Steven Rostedt (Red Hat)" <rostedt@goodmis.org>
> > > 
> > > [ REQUEST FOR ACKS ]
> > 
> > Can any of the netfilter folks give me an Acked-by for this?
> 
> If Florian's concern were addressed, then:

Yeah, the change he mentioned was done is 3/8. As that was written by
Joe Perches, I did some work that he missed and put it before his
patch, which showed a discrepancy between the two functions. After all
patches are applied, it should be consistent to his liking.

> 
> Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
> 
> to this patch and 4/8.

Thanks for this!

Can I get someone to ack 3/8?

Thanks again,

-- Steve

Re: [RFA][PATCH 2/8] netfilter: Remove return values for print_conntrack callbacks

[Joe Perches]

On Tue, 2014-11-04 at 09:31 -0500, Steven Rostedt wrote:
> On Tue, 4 Nov 2014 15:22:36 +0100
> Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > On Tue, Nov 04, 2014 at 08:05:35AM -0500, Steven Rostedt wrote:
> > > On Wed, 29 Oct 2014 17:56:04 -0400 Steven Rostedt <rostedt@goodmis.org> wrote:
> > > > From: "Steven Rostedt (Red Hat)" <rostedt@goodmis.org>
> > > > [ REQUEST FOR ACKS ]
> > > Can any of the netfilter folks give me an Acked-by for this?
> > If Florian's concern were addressed, then:
> Yeah, the change he mentioned was done is 3/8. As that was written by
> Joe Perches, I did some work that he missed and put it before his
> patch, which showed a discrepancy between the two functions. After all
> patches are applied, it should be consistent to his liking.

I think seq_has_overflowed does not need
to be used after every seq_<put/print> call.

It interrupts reading code flow and just
isn't alll that necessary as every operation
before it will be redone anyway.

It should be used before or after a large
blocks though.

Re: [RFA][PATCH 2/8] netfilter: Remove return values for print_conntrack callbacks

[Steven Rostedt]

On Tue, 04 Nov 2014 06:46:19 -0800
Joe Perches <joe@perches.com> wrote:

> On Tue, 2014-11-04 at 09:31 -0500, Steven Rostedt wrote:
> > On Tue, 4 Nov 2014 15:22:36 +0100
> > Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > > On Tue, Nov 04, 2014 at 08:05:35AM -0500, Steven Rostedt wrote:
> > > > On Wed, 29 Oct 2014 17:56:04 -0400 Steven Rostedt <rostedt@goodmis.org> wrote:
> > > > > From: "Steven Rostedt (Red Hat)" <rostedt@goodmis.org>
> > > > > [ REQUEST FOR ACKS ]
> > > > Can any of the netfilter folks give me an Acked-by for this?
> > > If Florian's concern were addressed, then:
> > Yeah, the change he mentioned was done is 3/8. As that was written by
> > Joe Perches, I did some work that he missed and put it before his
> > patch, which showed a discrepancy between the two functions. After all
> > patches are applied, it should be consistent to his liking.
> 
> I think seq_has_overflowed does not need
> to be used after every seq_<put/print> call.
> 
> It interrupts reading code flow and just
> isn't alll that necessary as every operation
> before it will be redone anyway.
> 
> It should be used before or after a large
> blocks though.
> 

It's not used in every occurrence. The problem that Florian had was that
there were two almost identical functions, and you changed one to have
the seq_has_overflowed() check, but the other one was left without it.

It wasn't about checking multiple times, it was about consistency
between two similar functions.

-- Steve

Re: [RFA][PATCH 2/8] netfilter: Remove return values for print_conntrack callbacks

[Pablo Neira Ayuso]

On Tue, Nov 04, 2014 at 09:31:50AM -0500, Steven Rostedt wrote:
> On Tue, 4 Nov 2014 15:22:36 +0100
> Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> 
> > On Tue, Nov 04, 2014 at 08:05:35AM -0500, Steven Rostedt wrote:
> > > On Wed, 29 Oct 2014 17:56:04 -0400
> > > Steven Rostedt <rostedt@goodmis.org> wrote:
> > > 
> > > > From: "Steven Rostedt (Red Hat)" <rostedt@goodmis.org>
> > > > 
> > > > [ REQUEST FOR ACKS ]
> > > 
> > > Can any of the netfilter folks give me an Acked-by for this?
> > 
> > If Florian's concern were addressed, then:
> 
> Yeah, the change he mentioned was done is 3/8. As that was written by
> Joe Perches, I did some work that he missed and put it before his
> patch, which showed a discrepancy between the two functions. After all
> patches are applied, it should be consistent to his liking.
> 
> > 
> > Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
> > 
> > to this patch and 4/8.
> 
> Thanks for this!
> 
> Can I get someone to ack 3/8?

I don't see any 3/8 in my inbox, only 2/8 and 4/8.

Let me know, thanks.

Re: [RFA][PATCH 2/8] netfilter: Remove return values for print_conntrack callbacks

[Steven Rostedt]

On Tue, 4 Nov 2014 15:46:32 +0100
Pablo Neira Ayuso <pablo@netfilter.org> wrote:


> > Can I get someone to ack 3/8?
> 
> I don't see any 3/8 in my inbox, only 2/8 and 4/8.
> 
> Let me know, thanks.

Strange, you were on the Cc of that patch.

Let me go back and check to make sure that didn't bounce too.

-- Steve

Re: [RFA][PATCH 2/8] netfilter: Remove return values for print_conntrack callbacks

[Steven Rostedt]

On Tue, 4 Nov 2014 15:46:32 +0100
Pablo Neira Ayuso <pablo@netfilter.org> wrote:


> > > 
> > > Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
> > > 
> > > to this patch and 4/8.
> > 
> > Thanks for this!
> > 
> > Can I get someone to ack 3/8?
> 
> I don't see any 3/8 in my inbox, only 2/8 and 4/8.
> 
> Let me know, thanks.

I replied to the patch. Did you get that reply?

Or did it somehow end up in your spam filter?

-- Steve

[RFA][PATCH 3/8] netfilter: Convert print_tuple functions to return void

[Steven Rostedt]

[-- Attachment #1: 0003-netfilter-Convert-print_tuple-functions-to-return-vo.patch --]
[-- Type: text/plain, Size: 16010 bytes --]

From: Joe Perches <joe@perches.com>

[ REQUEST FOR ACKS ]

Since adding a new function to seq_file (seq_has_overflowed())
there isn't any value for functions called from seq_show to
return anything.   Remove the int returns of the various
print_tuple/<foo>_print_tuple functions.

Link: http://lkml.kernel.org/p/f2e8cf8df433a197daa62cbaf124c900c708edc7.1412031505.git.joe@perches.com

Cc: Pablo Neira Ayuso <pablo@netfilter.org>
Cc: Patrick McHardy <kaber@trash.net>
Cc: Jozsef Kadlecsik <kadlec@blackhole.kfki.hu>
Cc: netfilter-devel@vger.kernel.org
Cc: coreteam@netfilter.org
Signed-off-by: Joe Perches <joe@perches.com>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 include/net/netfilter/nf_conntrack_core.h             |  2 +-
 include/net/netfilter/nf_conntrack_l3proto.h          |  4 ++--
 include/net/netfilter/nf_conntrack_l4proto.h          |  4 ++--
 net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c        |  6 +++---
 net/ipv4/netfilter/nf_conntrack_l3proto_ipv4_compat.c | 12 ++++++++----
 net/ipv4/netfilter/nf_conntrack_proto_icmp.c          | 10 +++++-----
 net/ipv6/netfilter/nf_conntrack_l3proto_ipv6.c        |  6 +++---
 net/ipv6/netfilter/nf_conntrack_proto_icmpv6.c        | 10 +++++-----
 net/netfilter/nf_conntrack_l3proto_generic.c          |  5 ++---
 net/netfilter/nf_conntrack_proto_dccp.c               | 10 +++++-----
 net/netfilter/nf_conntrack_proto_generic.c            |  5 ++---
 net/netfilter/nf_conntrack_proto_gre.c                | 10 +++++-----
 net/netfilter/nf_conntrack_proto_sctp.c               | 10 +++++-----
 net/netfilter/nf_conntrack_proto_tcp.c                | 10 +++++-----
 net/netfilter/nf_conntrack_proto_udp.c                | 10 +++++-----
 net/netfilter/nf_conntrack_proto_udplite.c            | 10 +++++-----
 net/netfilter/nf_conntrack_standalone.c               | 15 +++++++--------
 17 files changed, 70 insertions(+), 69 deletions(-)

diff --git a/include/net/netfilter/nf_conntrack_core.h b/include/net/netfilter/nf_conntrack_core.h
index cc0c18827602..f2f0fa3bb150 100644
--- a/include/net/netfilter/nf_conntrack_core.h
+++ b/include/net/netfilter/nf_conntrack_core.h
@@ -72,7 +72,7 @@ static inline int nf_conntrack_confirm(struct sk_buff *skb)
 	return ret;
 }
 
-int
+void
 print_tuple(struct seq_file *s, const struct nf_conntrack_tuple *tuple,
             const struct nf_conntrack_l3proto *l3proto,
             const struct nf_conntrack_l4proto *proto);
diff --git a/include/net/netfilter/nf_conntrack_l3proto.h b/include/net/netfilter/nf_conntrack_l3proto.h
index adc1fa3dd7ab..cdc920b4c4c2 100644
--- a/include/net/netfilter/nf_conntrack_l3proto.h
+++ b/include/net/netfilter/nf_conntrack_l3proto.h
@@ -38,8 +38,8 @@ struct nf_conntrack_l3proto {
 			     const struct nf_conntrack_tuple *orig);
 
 	/* Print out the per-protocol part of the tuple. */
-	int (*print_tuple)(struct seq_file *s,
-			   const struct nf_conntrack_tuple *);
+	void (*print_tuple)(struct seq_file *s,
+			    const struct nf_conntrack_tuple *);
 
 	/*
 	 * Called before tracking. 
diff --git a/include/net/netfilter/nf_conntrack_l4proto.h b/include/net/netfilter/nf_conntrack_l4proto.h
index 82e4ec002a39..1f7061313d54 100644
--- a/include/net/netfilter/nf_conntrack_l4proto.h
+++ b/include/net/netfilter/nf_conntrack_l4proto.h
@@ -56,8 +56,8 @@ struct nf_conntrack_l4proto {
 		     u_int8_t pf, unsigned int hooknum);
 
 	/* Print out the per-protocol part of the tuple. Return like seq_* */
-	int (*print_tuple)(struct seq_file *s,
-			   const struct nf_conntrack_tuple *);
+	void (*print_tuple)(struct seq_file *s,
+			    const struct nf_conntrack_tuple *);
 
 	/* Print out the private part of the conntrack. */
 	void (*print_conntrack)(struct seq_file *s, struct nf_conn *);
diff --git a/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c b/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c
index a054fe083431..5c61328b7704 100644
--- a/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c
+++ b/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c
@@ -56,11 +56,11 @@ static bool ipv4_invert_tuple(struct nf_conntrack_tuple *tuple,
 	return true;
 }
 
-static int ipv4_print_tuple(struct seq_file *s,
+static void ipv4_print_tuple(struct seq_file *s,
 			    const struct nf_conntrack_tuple *tuple)
 {
-	return seq_printf(s, "src=%pI4 dst=%pI4 ",
-			  &tuple->src.u3.ip, &tuple->dst.u3.ip);
+	seq_printf(s, "src=%pI4 dst=%pI4 ",
+		   &tuple->src.u3.ip, &tuple->dst.u3.ip);
 }
 
 static int ipv4_get_l4proto(const struct sk_buff *skb, unsigned int nhoff,
diff --git a/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4_compat.c b/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4_compat.c
index 91f207c2cb69..d927f9e72130 100644
--- a/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4_compat.c
+++ b/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4_compat.c
@@ -153,8 +153,10 @@ static int ct_seq_show(struct seq_file *s, void *v)
 	if (seq_has_overflowed(s))
 		goto release;
 
-	if (print_tuple(s, &ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple,
-			l3proto, l4proto))
+	print_tuple(s, &ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple,
+		    l3proto, l4proto);
+
+	if (seq_has_overflowed(s))
 		goto release;
 
 	if (seq_print_acct(s, ct, IP_CT_DIR_ORIGINAL))
@@ -164,8 +166,10 @@ static int ct_seq_show(struct seq_file *s, void *v)
 		if (seq_printf(s, "[UNREPLIED] "))
 			goto release;
 
-	if (print_tuple(s, &ct->tuplehash[IP_CT_DIR_REPLY].tuple,
-			l3proto, l4proto))
+	print_tuple(s, &ct->tuplehash[IP_CT_DIR_REPLY].tuple,
+		    l3proto, l4proto);
+
+	if (seq_has_overflowed(s))
 		goto release;
 
 	if (seq_print_acct(s, ct, IP_CT_DIR_REPLY))
diff --git a/net/ipv4/netfilter/nf_conntrack_proto_icmp.c b/net/ipv4/netfilter/nf_conntrack_proto_icmp.c
index b91b2641adda..80d5554b9a88 100644
--- a/net/ipv4/netfilter/nf_conntrack_proto_icmp.c
+++ b/net/ipv4/netfilter/nf_conntrack_proto_icmp.c
@@ -72,13 +72,13 @@ static bool icmp_invert_tuple(struct nf_conntrack_tuple *tuple,
 }
 
 /* Print out the per-protocol part of the tuple. */
-static int icmp_print_tuple(struct seq_file *s,
+static void icmp_print_tuple(struct seq_file *s,
 			    const struct nf_conntrack_tuple *tuple)
 {
-	return seq_printf(s, "type=%u code=%u id=%u ",
-			  tuple->dst.u.icmp.type,
-			  tuple->dst.u.icmp.code,
-			  ntohs(tuple->src.u.icmp.id));
+	seq_printf(s, "type=%u code=%u id=%u ",
+		   tuple->dst.u.icmp.type,
+		   tuple->dst.u.icmp.code,
+		   ntohs(tuple->src.u.icmp.id));
 }
 
 static unsigned int *icmp_get_timeouts(struct net *net)
diff --git a/net/ipv6/netfilter/nf_conntrack_l3proto_ipv6.c b/net/ipv6/netfilter/nf_conntrack_l3proto_ipv6.c
index 4cbc6b290dd5..b68d0e59c1f8 100644
--- a/net/ipv6/netfilter/nf_conntrack_l3proto_ipv6.c
+++ b/net/ipv6/netfilter/nf_conntrack_l3proto_ipv6.c
@@ -60,11 +60,11 @@ static bool ipv6_invert_tuple(struct nf_conntrack_tuple *tuple,
 	return true;
 }
 
-static int ipv6_print_tuple(struct seq_file *s,
+static void ipv6_print_tuple(struct seq_file *s,
 			    const struct nf_conntrack_tuple *tuple)
 {
-	return seq_printf(s, "src=%pI6 dst=%pI6 ",
-			  tuple->src.u3.ip6, tuple->dst.u3.ip6);
+	seq_printf(s, "src=%pI6 dst=%pI6 ",
+		   tuple->src.u3.ip6, tuple->dst.u3.ip6);
 }
 
 static int ipv6_get_l4proto(const struct sk_buff *skb, unsigned int nhoff,
diff --git a/net/ipv6/netfilter/nf_conntrack_proto_icmpv6.c b/net/ipv6/netfilter/nf_conntrack_proto_icmpv6.c
index b3807c5cb888..90388d606483 100644
--- a/net/ipv6/netfilter/nf_conntrack_proto_icmpv6.c
+++ b/net/ipv6/netfilter/nf_conntrack_proto_icmpv6.c
@@ -84,13 +84,13 @@ static bool icmpv6_invert_tuple(struct nf_conntrack_tuple *tuple,
 }
 
 /* Print out the per-protocol part of the tuple. */
-static int icmpv6_print_tuple(struct seq_file *s,
+static void icmpv6_print_tuple(struct seq_file *s,
 			      const struct nf_conntrack_tuple *tuple)
 {
-	return seq_printf(s, "type=%u code=%u id=%u ",
-			  tuple->dst.u.icmp.type,
-			  tuple->dst.u.icmp.code,
-			  ntohs(tuple->src.u.icmp.id));
+	seq_printf(s, "type=%u code=%u id=%u ",
+		   tuple->dst.u.icmp.type,
+		   tuple->dst.u.icmp.code,
+		   ntohs(tuple->src.u.icmp.id));
 }
 
 static unsigned int *icmpv6_get_timeouts(struct net *net)
diff --git a/net/netfilter/nf_conntrack_l3proto_generic.c b/net/netfilter/nf_conntrack_l3proto_generic.c
index e7eb807fe07d..cf9ace70bece 100644
--- a/net/netfilter/nf_conntrack_l3proto_generic.c
+++ b/net/netfilter/nf_conntrack_l3proto_generic.c
@@ -49,10 +49,9 @@ static bool generic_invert_tuple(struct nf_conntrack_tuple *tuple,
 	return true;
 }
 
-static int generic_print_tuple(struct seq_file *s,
-			    const struct nf_conntrack_tuple *tuple)
+static void generic_print_tuple(struct seq_file *s,
+				const struct nf_conntrack_tuple *tuple)
 {
-	return 0;
 }
 
 static int generic_get_l4proto(const struct sk_buff *skb, unsigned int nhoff,
diff --git a/net/netfilter/nf_conntrack_proto_dccp.c b/net/netfilter/nf_conntrack_proto_dccp.c
index 15971177470a..6dd995c7c72b 100644
--- a/net/netfilter/nf_conntrack_proto_dccp.c
+++ b/net/netfilter/nf_conntrack_proto_dccp.c
@@ -618,12 +618,12 @@ out_invalid:
 	return -NF_ACCEPT;
 }
 
-static int dccp_print_tuple(struct seq_file *s,
-			    const struct nf_conntrack_tuple *tuple)
+static void dccp_print_tuple(struct seq_file *s,
+			     const struct nf_conntrack_tuple *tuple)
 {
-	return seq_printf(s, "sport=%hu dport=%hu ",
-			  ntohs(tuple->src.u.dccp.port),
-			  ntohs(tuple->dst.u.dccp.port));
+	seq_printf(s, "sport=%hu dport=%hu ",
+		   ntohs(tuple->src.u.dccp.port),
+		   ntohs(tuple->dst.u.dccp.port));
 }
 
 static void dccp_print_conntrack(struct seq_file *s, struct nf_conn *ct)
diff --git a/net/netfilter/nf_conntrack_proto_generic.c b/net/netfilter/nf_conntrack_proto_generic.c
index 957c1db66652..60865f110309 100644
--- a/net/netfilter/nf_conntrack_proto_generic.c
+++ b/net/netfilter/nf_conntrack_proto_generic.c
@@ -63,10 +63,9 @@ static bool generic_invert_tuple(struct nf_conntrack_tuple *tuple,
 }
 
 /* Print out the per-protocol part of the tuple. */
-static int generic_print_tuple(struct seq_file *s,
-			       const struct nf_conntrack_tuple *tuple)
+static void generic_print_tuple(struct seq_file *s,
+				const struct nf_conntrack_tuple *tuple)
 {
-	return 0;
 }
 
 static unsigned int *generic_get_timeouts(struct net *net)
diff --git a/net/netfilter/nf_conntrack_proto_gre.c b/net/netfilter/nf_conntrack_proto_gre.c
index cba607ada069..7648674f29c3 100644
--- a/net/netfilter/nf_conntrack_proto_gre.c
+++ b/net/netfilter/nf_conntrack_proto_gre.c
@@ -226,12 +226,12 @@ static bool gre_pkt_to_tuple(const struct sk_buff *skb, unsigned int dataoff,
 }
 
 /* print gre part of tuple */
-static int gre_print_tuple(struct seq_file *s,
-			   const struct nf_conntrack_tuple *tuple)
+static void gre_print_tuple(struct seq_file *s,
+			    const struct nf_conntrack_tuple *tuple)
 {
-	return seq_printf(s, "srckey=0x%x dstkey=0x%x ",
-			  ntohs(tuple->src.u.gre.key),
-			  ntohs(tuple->dst.u.gre.key));
+	seq_printf(s, "srckey=0x%x dstkey=0x%x ",
+		   ntohs(tuple->src.u.gre.key),
+		   ntohs(tuple->dst.u.gre.key));
 }
 
 /* print private data for conntrack */
diff --git a/net/netfilter/nf_conntrack_proto_sctp.c b/net/netfilter/nf_conntrack_proto_sctp.c
index c61f4cd6407d..b45da90fad32 100644
--- a/net/netfilter/nf_conntrack_proto_sctp.c
+++ b/net/netfilter/nf_conntrack_proto_sctp.c
@@ -166,12 +166,12 @@ static bool sctp_invert_tuple(struct nf_conntrack_tuple *tuple,
 }
 
 /* Print out the per-protocol part of the tuple. */
-static int sctp_print_tuple(struct seq_file *s,
-			    const struct nf_conntrack_tuple *tuple)
+static void sctp_print_tuple(struct seq_file *s,
+			     const struct nf_conntrack_tuple *tuple)
 {
-	return seq_printf(s, "sport=%hu dport=%hu ",
-			  ntohs(tuple->src.u.sctp.port),
-			  ntohs(tuple->dst.u.sctp.port));
+	seq_printf(s, "sport=%hu dport=%hu ",
+		   ntohs(tuple->src.u.sctp.port),
+		   ntohs(tuple->dst.u.sctp.port));
 }
 
 /* Print out the private part of the conntrack. */
diff --git a/net/netfilter/nf_conntrack_proto_tcp.c b/net/netfilter/nf_conntrack_proto_tcp.c
index 79668fd3db96..36a3ac8ee9f5 100644
--- a/net/netfilter/nf_conntrack_proto_tcp.c
+++ b/net/netfilter/nf_conntrack_proto_tcp.c
@@ -302,12 +302,12 @@ static bool tcp_invert_tuple(struct nf_conntrack_tuple *tuple,
 }
 
 /* Print out the per-protocol part of the tuple. */
-static int tcp_print_tuple(struct seq_file *s,
-			   const struct nf_conntrack_tuple *tuple)
+static void tcp_print_tuple(struct seq_file *s,
+			    const struct nf_conntrack_tuple *tuple)
 {
-	return seq_printf(s, "sport=%hu dport=%hu ",
-			  ntohs(tuple->src.u.tcp.port),
-			  ntohs(tuple->dst.u.tcp.port));
+	seq_printf(s, "sport=%hu dport=%hu ",
+		   ntohs(tuple->src.u.tcp.port),
+		   ntohs(tuple->dst.u.tcp.port));
 }
 
 /* Print out the private part of the conntrack. */
diff --git a/net/netfilter/nf_conntrack_proto_udp.c b/net/netfilter/nf_conntrack_proto_udp.c
index 9d7721cbce4b..6957281ffee5 100644
--- a/net/netfilter/nf_conntrack_proto_udp.c
+++ b/net/netfilter/nf_conntrack_proto_udp.c
@@ -63,12 +63,12 @@ static bool udp_invert_tuple(struct nf_conntrack_tuple *tuple,
 }
 
 /* Print out the per-protocol part of the tuple. */
-static int udp_print_tuple(struct seq_file *s,
-			   const struct nf_conntrack_tuple *tuple)
+static void udp_print_tuple(struct seq_file *s,
+			    const struct nf_conntrack_tuple *tuple)
 {
-	return seq_printf(s, "sport=%hu dport=%hu ",
-			  ntohs(tuple->src.u.udp.port),
-			  ntohs(tuple->dst.u.udp.port));
+	seq_printf(s, "sport=%hu dport=%hu ",
+		   ntohs(tuple->src.u.udp.port),
+		   ntohs(tuple->dst.u.udp.port));
 }
 
 static unsigned int *udp_get_timeouts(struct net *net)
diff --git a/net/netfilter/nf_conntrack_proto_udplite.c b/net/netfilter/nf_conntrack_proto_udplite.c
index 2750e6c69f82..c5903d1649f9 100644
--- a/net/netfilter/nf_conntrack_proto_udplite.c
+++ b/net/netfilter/nf_conntrack_proto_udplite.c
@@ -71,12 +71,12 @@ static bool udplite_invert_tuple(struct nf_conntrack_tuple *tuple,
 }
 
 /* Print out the per-protocol part of the tuple. */
-static int udplite_print_tuple(struct seq_file *s,
-			       const struct nf_conntrack_tuple *tuple)
+static void udplite_print_tuple(struct seq_file *s,
+				const struct nf_conntrack_tuple *tuple)
 {
-	return seq_printf(s, "sport=%hu dport=%hu ",
-			  ntohs(tuple->src.u.udp.port),
-			  ntohs(tuple->dst.u.udp.port));
+	seq_printf(s, "sport=%hu dport=%hu ",
+		   ntohs(tuple->src.u.udp.port),
+		   ntohs(tuple->dst.u.udp.port));
 }
 
 static unsigned int *udplite_get_timeouts(struct net *net)
diff --git a/net/netfilter/nf_conntrack_standalone.c b/net/netfilter/nf_conntrack_standalone.c
index 348aa3602787..23a0dcab21d4 100644
--- a/net/netfilter/nf_conntrack_standalone.c
+++ b/net/netfilter/nf_conntrack_standalone.c
@@ -36,12 +36,13 @@
 MODULE_LICENSE("GPL");
 
 #ifdef CONFIG_NF_CONNTRACK_PROCFS
-int
+void
 print_tuple(struct seq_file *s, const struct nf_conntrack_tuple *tuple,
             const struct nf_conntrack_l3proto *l3proto,
             const struct nf_conntrack_l4proto *l4proto)
 {
-	return l3proto->print_tuple(s, tuple) || l4proto->print_tuple(s, tuple);
+	l3proto->print_tuple(s, tuple);
+	l4proto->print_tuple(s, tuple);
 }
 EXPORT_SYMBOL_GPL(print_tuple);
 
@@ -202,9 +203,8 @@ static int ct_seq_show(struct seq_file *s, void *v)
 	if (l4proto->print_conntrack)
 		l4proto->print_conntrack(s, ct);
 
-	if (print_tuple(s, &ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple,
-			l3proto, l4proto))
-		goto release;
+	print_tuple(s, &ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple,
+		    l3proto, l4proto);
 
 	if (seq_print_acct(s, ct, IP_CT_DIR_ORIGINAL))
 		goto release;
@@ -213,9 +213,8 @@ static int ct_seq_show(struct seq_file *s, void *v)
 		if (seq_printf(s, "[UNREPLIED] "))
 			goto release;
 
-	if (print_tuple(s, &ct->tuplehash[IP_CT_DIR_REPLY].tuple,
-			l3proto, l4proto))
-		goto release;
+	print_tuple(s, &ct->tuplehash[IP_CT_DIR_REPLY].tuple,
+		    l3proto, l4proto);
 
 	if (seq_print_acct(s, ct, IP_CT_DIR_REPLY))
 		goto release;
-- 
2.1.1

Re: [RFA][PATCH 3/8] netfilter: Convert print_tuple functions to return void

[Steven Rostedt]

On Wed, 29 Oct 2014 17:56:05 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

> From: Joe Perches <joe@perches.com>
> 
> [ REQUEST FOR ACKS ]

Can any of the netfilter maintainers give me an Acked-by for this?

Thanks!

-- Steve

> 
> Since adding a new function to seq_file (seq_has_overflowed())
> there isn't any value for functions called from seq_show to
> return anything.   Remove the int returns of the various
> print_tuple/<foo>_print_tuple functions.
> 
> Link: http://lkml.kernel.org/p/f2e8cf8df433a197daa62cbaf124c900c708edc7.1412031505.git.joe@perches.com
> 
> Cc: Pablo Neira Ayuso <pablo@netfilter.org>
> Cc: Patrick McHardy <kaber@trash.net>
> Cc: Jozsef Kadlecsik <kadlec@blackhole.kfki.hu>
> Cc: netfilter-devel@vger.kernel.org
> Cc: coreteam@netfilter.org
> Signed-off-by: Joe Perches <joe@perches.com>
> Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
> ---
>  include/net/netfilter/nf_conntrack_core.h             |  2 +-
>  include/net/netfilter/nf_conntrack_l3proto.h          |  4 ++--
>  include/net/netfilter/nf_conntrack_l4proto.h          |  4 ++--
>  net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c        |  6 +++---
>  net/ipv4/netfilter/nf_conntrack_l3proto_ipv4_compat.c | 12 ++++++++----
>  net/ipv4/netfilter/nf_conntrack_proto_icmp.c          | 10 +++++-----
>  net/ipv6/netfilter/nf_conntrack_l3proto_ipv6.c        |  6 +++---
>  net/ipv6/netfilter/nf_conntrack_proto_icmpv6.c        | 10 +++++-----
>  net/netfilter/nf_conntrack_l3proto_generic.c          |  5 ++---
>  net/netfilter/nf_conntrack_proto_dccp.c               | 10 +++++-----
>  net/netfilter/nf_conntrack_proto_generic.c            |  5 ++---
>  net/netfilter/nf_conntrack_proto_gre.c                | 10 +++++-----
>  net/netfilter/nf_conntrack_proto_sctp.c               | 10 +++++-----
>  net/netfilter/nf_conntrack_proto_tcp.c                | 10 +++++-----
>  net/netfilter/nf_conntrack_proto_udp.c                | 10 +++++-----
>  net/netfilter/nf_conntrack_proto_udplite.c            | 10 +++++-----
>  net/netfilter/nf_conntrack_standalone.c               | 15 +++++++--------
>  17 files changed, 70 insertions(+), 69 deletions(-)
> 
> diff --git a/include/net/netfilter/nf_conntrack_core.h b/include/net/netfilter/nf_conntrack_core.h
> index cc0c18827602..f2f0fa3bb150 100644
> --- a/include/net/netfilter/nf_conntrack_core.h
> +++ b/include/net/netfilter/nf_conntrack_core.h
> @@ -72,7 +72,7 @@ static inline int nf_conntrack_confirm(struct sk_buff *skb)
>  	return ret;
>  }
>  
> -int
> +void
>  print_tuple(struct seq_file *s, const struct nf_conntrack_tuple *tuple,
>              const struct nf_conntrack_l3proto *l3proto,
>              const struct nf_conntrack_l4proto *proto);
> diff --git a/include/net/netfilter/nf_conntrack_l3proto.h b/include/net/netfilter/nf_conntrack_l3proto.h
> index adc1fa3dd7ab..cdc920b4c4c2 100644
> --- a/include/net/netfilter/nf_conntrack_l3proto.h
> +++ b/include/net/netfilter/nf_conntrack_l3proto.h
> @@ -38,8 +38,8 @@ struct nf_conntrack_l3proto {
>  			     const struct nf_conntrack_tuple *orig);
>  
>  	/* Print out the per-protocol part of the tuple. */
> -	int (*print_tuple)(struct seq_file *s,
> -			   const struct nf_conntrack_tuple *);
> +	void (*print_tuple)(struct seq_file *s,
> +			    const struct nf_conntrack_tuple *);
>  
>  	/*
>  	 * Called before tracking. 
> diff --git a/include/net/netfilter/nf_conntrack_l4proto.h b/include/net/netfilter/nf_conntrack_l4proto.h
> index 82e4ec002a39..1f7061313d54 100644
> --- a/include/net/netfilter/nf_conntrack_l4proto.h
> +++ b/include/net/netfilter/nf_conntrack_l4proto.h
> @@ -56,8 +56,8 @@ struct nf_conntrack_l4proto {
>  		     u_int8_t pf, unsigned int hooknum);
>  
>  	/* Print out the per-protocol part of the tuple. Return like seq_* */
> -	int (*print_tuple)(struct seq_file *s,
> -			   const struct nf_conntrack_tuple *);
> +	void (*print_tuple)(struct seq_file *s,
> +			    const struct nf_conntrack_tuple *);
>  
>  	/* Print out the private part of the conntrack. */
>  	void (*print_conntrack)(struct seq_file *s, struct nf_conn *);
> diff --git a/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c b/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c
> index a054fe083431..5c61328b7704 100644
> --- a/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c
> +++ b/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c
> @@ -56,11 +56,11 @@ static bool ipv4_invert_tuple(struct nf_conntrack_tuple *tuple,
>  	return true;
>  }
>  
> -static int ipv4_print_tuple(struct seq_file *s,
> +static void ipv4_print_tuple(struct seq_file *s,
>  			    const struct nf_conntrack_tuple *tuple)
>  {
> -	return seq_printf(s, "src=%pI4 dst=%pI4 ",
> -			  &tuple->src.u3.ip, &tuple->dst.u3.ip);
> +	seq_printf(s, "src=%pI4 dst=%pI4 ",
> +		   &tuple->src.u3.ip, &tuple->dst.u3.ip);
>  }
>  
>  static int ipv4_get_l4proto(const struct sk_buff *skb, unsigned int nhoff,
> diff --git a/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4_compat.c b/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4_compat.c
> index 91f207c2cb69..d927f9e72130 100644
> --- a/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4_compat.c
> +++ b/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4_compat.c
> @@ -153,8 +153,10 @@ static int ct_seq_show(struct seq_file *s, void *v)
>  	if (seq_has_overflowed(s))
>  		goto release;
>  
> -	if (print_tuple(s, &ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple,
> -			l3proto, l4proto))
> +	print_tuple(s, &ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple,
> +		    l3proto, l4proto);
> +
> +	if (seq_has_overflowed(s))
>  		goto release;
>  
>  	if (seq_print_acct(s, ct, IP_CT_DIR_ORIGINAL))
> @@ -164,8 +166,10 @@ static int ct_seq_show(struct seq_file *s, void *v)
>  		if (seq_printf(s, "[UNREPLIED] "))
>  			goto release;
>  
> -	if (print_tuple(s, &ct->tuplehash[IP_CT_DIR_REPLY].tuple,
> -			l3proto, l4proto))
> +	print_tuple(s, &ct->tuplehash[IP_CT_DIR_REPLY].tuple,
> +		    l3proto, l4proto);
> +
> +	if (seq_has_overflowed(s))
>  		goto release;
>  
>  	if (seq_print_acct(s, ct, IP_CT_DIR_REPLY))
> diff --git a/net/ipv4/netfilter/nf_conntrack_proto_icmp.c b/net/ipv4/netfilter/nf_conntrack_proto_icmp.c
> index b91b2641adda..80d5554b9a88 100644
> --- a/net/ipv4/netfilter/nf_conntrack_proto_icmp.c
> +++ b/net/ipv4/netfilter/nf_conntrack_proto_icmp.c
> @@ -72,13 +72,13 @@ static bool icmp_invert_tuple(struct nf_conntrack_tuple *tuple,
>  }
>  
>  /* Print out the per-protocol part of the tuple. */
> -static int icmp_print_tuple(struct seq_file *s,
> +static void icmp_print_tuple(struct seq_file *s,
>  			    const struct nf_conntrack_tuple *tuple)
>  {
> -	return seq_printf(s, "type=%u code=%u id=%u ",
> -			  tuple->dst.u.icmp.type,
> -			  tuple->dst.u.icmp.code,
> -			  ntohs(tuple->src.u.icmp.id));
> +	seq_printf(s, "type=%u code=%u id=%u ",
> +		   tuple->dst.u.icmp.type,
> +		   tuple->dst.u.icmp.code,
> +		   ntohs(tuple->src.u.icmp.id));
>  }
>  
>  static unsigned int *icmp_get_timeouts(struct net *net)
> diff --git a/net/ipv6/netfilter/nf_conntrack_l3proto_ipv6.c b/net/ipv6/netfilter/nf_conntrack_l3proto_ipv6.c
> index 4cbc6b290dd5..b68d0e59c1f8 100644
> --- a/net/ipv6/netfilter/nf_conntrack_l3proto_ipv6.c
> +++ b/net/ipv6/netfilter/nf_conntrack_l3proto_ipv6.c
> @@ -60,11 +60,11 @@ static bool ipv6_invert_tuple(struct nf_conntrack_tuple *tuple,
>  	return true;
>  }
>  
> -static int ipv6_print_tuple(struct seq_file *s,
> +static void ipv6_print_tuple(struct seq_file *s,
>  			    const struct nf_conntrack_tuple *tuple)
>  {
> -	return seq_printf(s, "src=%pI6 dst=%pI6 ",
> -			  tuple->src.u3.ip6, tuple->dst.u3.ip6);
> +	seq_printf(s, "src=%pI6 dst=%pI6 ",
> +		   tuple->src.u3.ip6, tuple->dst.u3.ip6);
>  }
>  
>  static int ipv6_get_l4proto(const struct sk_buff *skb, unsigned int nhoff,
> diff --git a/net/ipv6/netfilter/nf_conntrack_proto_icmpv6.c b/net/ipv6/netfilter/nf_conntrack_proto_icmpv6.c
> index b3807c5cb888..90388d606483 100644
> --- a/net/ipv6/netfilter/nf_conntrack_proto_icmpv6.c
> +++ b/net/ipv6/netfilter/nf_conntrack_proto_icmpv6.c
> @@ -84,13 +84,13 @@ static bool icmpv6_invert_tuple(struct nf_conntrack_tuple *tuple,
>  }
>  
>  /* Print out the per-protocol part of the tuple. */
> -static int icmpv6_print_tuple(struct seq_file *s,
> +static void icmpv6_print_tuple(struct seq_file *s,
>  			      const struct nf_conntrack_tuple *tuple)
>  {
> -	return seq_printf(s, "type=%u code=%u id=%u ",
> -			  tuple->dst.u.icmp.type,
> -			  tuple->dst.u.icmp.code,
> -			  ntohs(tuple->src.u.icmp.id));
> +	seq_printf(s, "type=%u code=%u id=%u ",
> +		   tuple->dst.u.icmp.type,
> +		   tuple->dst.u.icmp.code,
> +		   ntohs(tuple->src.u.icmp.id));
>  }
>  
>  static unsigned int *icmpv6_get_timeouts(struct net *net)
> diff --git a/net/netfilter/nf_conntrack_l3proto_generic.c b/net/netfilter/nf_conntrack_l3proto_generic.c
> index e7eb807fe07d..cf9ace70bece 100644
> --- a/net/netfilter/nf_conntrack_l3proto_generic.c
> +++ b/net/netfilter/nf_conntrack_l3proto_generic.c
> @@ -49,10 +49,9 @@ static bool generic_invert_tuple(struct nf_conntrack_tuple *tuple,
>  	return true;
>  }
>  
> -static int generic_print_tuple(struct seq_file *s,
> -			    const struct nf_conntrack_tuple *tuple)
> +static void generic_print_tuple(struct seq_file *s,
> +				const struct nf_conntrack_tuple *tuple)
>  {
> -	return 0;
>  }
>  
>  static int generic_get_l4proto(const struct sk_buff *skb, unsigned int nhoff,
> diff --git a/net/netfilter/nf_conntrack_proto_dccp.c b/net/netfilter/nf_conntrack_proto_dccp.c
> index 15971177470a..6dd995c7c72b 100644
> --- a/net/netfilter/nf_conntrack_proto_dccp.c
> +++ b/net/netfilter/nf_conntrack_proto_dccp.c
> @@ -618,12 +618,12 @@ out_invalid:
>  	return -NF_ACCEPT;
>  }
>  
> -static int dccp_print_tuple(struct seq_file *s,
> -			    const struct nf_conntrack_tuple *tuple)
> +static void dccp_print_tuple(struct seq_file *s,
> +			     const struct nf_conntrack_tuple *tuple)
>  {
> -	return seq_printf(s, "sport=%hu dport=%hu ",
> -			  ntohs(tuple->src.u.dccp.port),
> -			  ntohs(tuple->dst.u.dccp.port));
> +	seq_printf(s, "sport=%hu dport=%hu ",
> +		   ntohs(tuple->src.u.dccp.port),
> +		   ntohs(tuple->dst.u.dccp.port));
>  }
>  
>  static void dccp_print_conntrack(struct seq_file *s, struct nf_conn *ct)
> diff --git a/net/netfilter/nf_conntrack_proto_generic.c b/net/netfilter/nf_conntrack_proto_generic.c
> index 957c1db66652..60865f110309 100644
> --- a/net/netfilter/nf_conntrack_proto_generic.c
> +++ b/net/netfilter/nf_conntrack_proto_generic.c
> @@ -63,10 +63,9 @@ static bool generic_invert_tuple(struct nf_conntrack_tuple *tuple,
>  }
>  
>  /* Print out the per-protocol part of the tuple. */
> -static int generic_print_tuple(struct seq_file *s,
> -			       const struct nf_conntrack_tuple *tuple)
> +static void generic_print_tuple(struct seq_file *s,
> +				const struct nf_conntrack_tuple *tuple)
>  {
> -	return 0;
>  }
>  
>  static unsigned int *generic_get_timeouts(struct net *net)
> diff --git a/net/netfilter/nf_conntrack_proto_gre.c b/net/netfilter/nf_conntrack_proto_gre.c
> index cba607ada069..7648674f29c3 100644
> --- a/net/netfilter/nf_conntrack_proto_gre.c
> +++ b/net/netfilter/nf_conntrack_proto_gre.c
> @@ -226,12 +226,12 @@ static bool gre_pkt_to_tuple(const struct sk_buff *skb, unsigned int dataoff,
>  }
>  
>  /* print gre part of tuple */
> -static int gre_print_tuple(struct seq_file *s,
> -			   const struct nf_conntrack_tuple *tuple)
> +static void gre_print_tuple(struct seq_file *s,
> +			    const struct nf_conntrack_tuple *tuple)
>  {
> -	return seq_printf(s, "srckey=0x%x dstkey=0x%x ",
> -			  ntohs(tuple->src.u.gre.key),
> -			  ntohs(tuple->dst.u.gre.key));
> +	seq_printf(s, "srckey=0x%x dstkey=0x%x ",
> +		   ntohs(tuple->src.u.gre.key),
> +		   ntohs(tuple->dst.u.gre.key));
>  }
>  
>  /* print private data for conntrack */
> diff --git a/net/netfilter/nf_conntrack_proto_sctp.c b/net/netfilter/nf_conntrack_proto_sctp.c
> index c61f4cd6407d..b45da90fad32 100644
> --- a/net/netfilter/nf_conntrack_proto_sctp.c
> +++ b/net/netfilter/nf_conntrack_proto_sctp.c
> @@ -166,12 +166,12 @@ static bool sctp_invert_tuple(struct nf_conntrack_tuple *tuple,
>  }
>  
>  /* Print out the per-protocol part of the tuple. */
> -static int sctp_print_tuple(struct seq_file *s,
> -			    const struct nf_conntrack_tuple *tuple)
> +static void sctp_print_tuple(struct seq_file *s,
> +			     const struct nf_conntrack_tuple *tuple)
>  {
> -	return seq_printf(s, "sport=%hu dport=%hu ",
> -			  ntohs(tuple->src.u.sctp.port),
> -			  ntohs(tuple->dst.u.sctp.port));
> +	seq_printf(s, "sport=%hu dport=%hu ",
> +		   ntohs(tuple->src.u.sctp.port),
> +		   ntohs(tuple->dst.u.sctp.port));
>  }
>  
>  /* Print out the private part of the conntrack. */
> diff --git a/net/netfilter/nf_conntrack_proto_tcp.c b/net/netfilter/nf_conntrack_proto_tcp.c
> index 79668fd3db96..36a3ac8ee9f5 100644
> --- a/net/netfilter/nf_conntrack_proto_tcp.c
> +++ b/net/netfilter/nf_conntrack_proto_tcp.c
> @@ -302,12 +302,12 @@ static bool tcp_invert_tuple(struct nf_conntrack_tuple *tuple,
>  }
>  
>  /* Print out the per-protocol part of the tuple. */
> -static int tcp_print_tuple(struct seq_file *s,
> -			   const struct nf_conntrack_tuple *tuple)
> +static void tcp_print_tuple(struct seq_file *s,
> +			    const struct nf_conntrack_tuple *tuple)
>  {
> -	return seq_printf(s, "sport=%hu dport=%hu ",
> -			  ntohs(tuple->src.u.tcp.port),
> -			  ntohs(tuple->dst.u.tcp.port));
> +	seq_printf(s, "sport=%hu dport=%hu ",
> +		   ntohs(tuple->src.u.tcp.port),
> +		   ntohs(tuple->dst.u.tcp.port));
>  }
>  
>  /* Print out the private part of the conntrack. */
> diff --git a/net/netfilter/nf_conntrack_proto_udp.c b/net/netfilter/nf_conntrack_proto_udp.c
> index 9d7721cbce4b..6957281ffee5 100644
> --- a/net/netfilter/nf_conntrack_proto_udp.c
> +++ b/net/netfilter/nf_conntrack_proto_udp.c
> @@ -63,12 +63,12 @@ static bool udp_invert_tuple(struct nf_conntrack_tuple *tuple,
>  }
>  
>  /* Print out the per-protocol part of the tuple. */
> -static int udp_print_tuple(struct seq_file *s,
> -			   const struct nf_conntrack_tuple *tuple)
> +static void udp_print_tuple(struct seq_file *s,
> +			    const struct nf_conntrack_tuple *tuple)
>  {
> -	return seq_printf(s, "sport=%hu dport=%hu ",
> -			  ntohs(tuple->src.u.udp.port),
> -			  ntohs(tuple->dst.u.udp.port));
> +	seq_printf(s, "sport=%hu dport=%hu ",
> +		   ntohs(tuple->src.u.udp.port),
> +		   ntohs(tuple->dst.u.udp.port));
>  }
>  
>  static unsigned int *udp_get_timeouts(struct net *net)
> diff --git a/net/netfilter/nf_conntrack_proto_udplite.c b/net/netfilter/nf_conntrack_proto_udplite.c
> index 2750e6c69f82..c5903d1649f9 100644
> --- a/net/netfilter/nf_conntrack_proto_udplite.c
> +++ b/net/netfilter/nf_conntrack_proto_udplite.c
> @@ -71,12 +71,12 @@ static bool udplite_invert_tuple(struct nf_conntrack_tuple *tuple,
>  }
>  
>  /* Print out the per-protocol part of the tuple. */
> -static int udplite_print_tuple(struct seq_file *s,
> -			       const struct nf_conntrack_tuple *tuple)
> +static void udplite_print_tuple(struct seq_file *s,
> +				const struct nf_conntrack_tuple *tuple)
>  {
> -	return seq_printf(s, "sport=%hu dport=%hu ",
> -			  ntohs(tuple->src.u.udp.port),
> -			  ntohs(tuple->dst.u.udp.port));
> +	seq_printf(s, "sport=%hu dport=%hu ",
> +		   ntohs(tuple->src.u.udp.port),
> +		   ntohs(tuple->dst.u.udp.port));
>  }
>  
>  static unsigned int *udplite_get_timeouts(struct net *net)
> diff --git a/net/netfilter/nf_conntrack_standalone.c b/net/netfilter/nf_conntrack_standalone.c
> index 348aa3602787..23a0dcab21d4 100644
> --- a/net/netfilter/nf_conntrack_standalone.c
> +++ b/net/netfilter/nf_conntrack_standalone.c
> @@ -36,12 +36,13 @@
>  MODULE_LICENSE("GPL");
>  
>  #ifdef CONFIG_NF_CONNTRACK_PROCFS
> -int
> +void
>  print_tuple(struct seq_file *s, const struct nf_conntrack_tuple *tuple,
>              const struct nf_conntrack_l3proto *l3proto,
>              const struct nf_conntrack_l4proto *l4proto)
>  {
> -	return l3proto->print_tuple(s, tuple) || l4proto->print_tuple(s, tuple);
> +	l3proto->print_tuple(s, tuple);
> +	l4proto->print_tuple(s, tuple);
>  }
>  EXPORT_SYMBOL_GPL(print_tuple);
>  
> @@ -202,9 +203,8 @@ static int ct_seq_show(struct seq_file *s, void *v)
>  	if (l4proto->print_conntrack)
>  		l4proto->print_conntrack(s, ct);
>  
> -	if (print_tuple(s, &ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple,
> -			l3proto, l4proto))
> -		goto release;
> +	print_tuple(s, &ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple,
> +		    l3proto, l4proto);
>  
>  	if (seq_print_acct(s, ct, IP_CT_DIR_ORIGINAL))
>  		goto release;
> @@ -213,9 +213,8 @@ static int ct_seq_show(struct seq_file *s, void *v)
>  		if (seq_printf(s, "[UNREPLIED] "))
>  			goto release;
>  
> -	if (print_tuple(s, &ct->tuplehash[IP_CT_DIR_REPLY].tuple,
> -			l3proto, l4proto))
> -		goto release;
> +	print_tuple(s, &ct->tuplehash[IP_CT_DIR_REPLY].tuple,
> +		    l3proto, l4proto);
>  
>  	if (seq_print_acct(s, ct, IP_CT_DIR_REPLY))
>  		goto release;

Re: [RFA][PATCH 3/8] netfilter: Convert print_tuple functions to return void

[Steven Rostedt]

Pablo,

I'm replying to the patch that was sent. Hopefully you get it.

Thanks,

-- Steve


On Wed, 29 Oct 2014 17:56:05 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

> From: Joe Perches <joe@perches.com>
> 
> [ REQUEST FOR ACKS ]
> 
> Since adding a new function to seq_file (seq_has_overflowed())
> there isn't any value for functions called from seq_show to
> return anything.   Remove the int returns of the various
> print_tuple/<foo>_print_tuple functions.
> 
> Link: http://lkml.kernel.org/p/f2e8cf8df433a197daa62cbaf124c900c708edc7.1412031505.git.joe@perches.com
> 
> Cc: Pablo Neira Ayuso <pablo@netfilter.org>
> Cc: Patrick McHardy <kaber@trash.net>
> Cc: Jozsef Kadlecsik <kadlec@blackhole.kfki.hu>
> Cc: netfilter-devel@vger.kernel.org
> Cc: coreteam@netfilter.org
> Signed-off-by: Joe Perches <joe@perches.com>
> Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
> ---
>  include/net/netfilter/nf_conntrack_core.h             |  2 +-
>  include/net/netfilter/nf_conntrack_l3proto.h          |  4 ++--
>  include/net/netfilter/nf_conntrack_l4proto.h          |  4 ++--
>  net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c        |  6 +++---
>  net/ipv4/netfilter/nf_conntrack_l3proto_ipv4_compat.c | 12 ++++++++----
>  net/ipv4/netfilter/nf_conntrack_proto_icmp.c          | 10 +++++-----
>  net/ipv6/netfilter/nf_conntrack_l3proto_ipv6.c        |  6 +++---
>  net/ipv6/netfilter/nf_conntrack_proto_icmpv6.c        | 10 +++++-----
>  net/netfilter/nf_conntrack_l3proto_generic.c          |  5 ++---
>  net/netfilter/nf_conntrack_proto_dccp.c               | 10 +++++-----
>  net/netfilter/nf_conntrack_proto_generic.c            |  5 ++---
>  net/netfilter/nf_conntrack_proto_gre.c                | 10 +++++-----
>  net/netfilter/nf_conntrack_proto_sctp.c               | 10 +++++-----
>  net/netfilter/nf_conntrack_proto_tcp.c                | 10 +++++-----
>  net/netfilter/nf_conntrack_proto_udp.c                | 10 +++++-----
>  net/netfilter/nf_conntrack_proto_udplite.c            | 10 +++++-----
>  net/netfilter/nf_conntrack_standalone.c               | 15 +++++++--------
>  17 files changed, 70 insertions(+), 69 deletions(-)
> 
> diff --git a/include/net/netfilter/nf_conntrack_core.h b/include/net/netfilter/nf_conntrack_core.h
> index cc0c18827602..f2f0fa3bb150 100644
> --- a/include/net/netfilter/nf_conntrack_core.h
> +++ b/include/net/netfilter/nf_conntrack_core.h
> @@ -72,7 +72,7 @@ static inline int nf_conntrack_confirm(struct sk_buff *skb)
>  	return ret;
>  }
>  
> -int
> +void
>  print_tuple(struct seq_file *s, const struct nf_conntrack_tuple *tuple,
>              const struct nf_conntrack_l3proto *l3proto,
>              const struct nf_conntrack_l4proto *proto);
> diff --git a/include/net/netfilter/nf_conntrack_l3proto.h b/include/net/netfilter/nf_conntrack_l3proto.h
> index adc1fa3dd7ab..cdc920b4c4c2 100644
> --- a/include/net/netfilter/nf_conntrack_l3proto.h
> +++ b/include/net/netfilter/nf_conntrack_l3proto.h
> @@ -38,8 +38,8 @@ struct nf_conntrack_l3proto {
>  			     const struct nf_conntrack_tuple *orig);
>  
>  	/* Print out the per-protocol part of the tuple. */
> -	int (*print_tuple)(struct seq_file *s,
> -			   const struct nf_conntrack_tuple *);
> +	void (*print_tuple)(struct seq_file *s,
> +			    const struct nf_conntrack_tuple *);
>  
>  	/*
>  	 * Called before tracking. 
> diff --git a/include/net/netfilter/nf_conntrack_l4proto.h b/include/net/netfilter/nf_conntrack_l4proto.h
> index 82e4ec002a39..1f7061313d54 100644
> --- a/include/net/netfilter/nf_conntrack_l4proto.h
> +++ b/include/net/netfilter/nf_conntrack_l4proto.h
> @@ -56,8 +56,8 @@ struct nf_conntrack_l4proto {
>  		     u_int8_t pf, unsigned int hooknum);
>  
>  	/* Print out the per-protocol part of the tuple. Return like seq_* */
> -	int (*print_tuple)(struct seq_file *s,
> -			   const struct nf_conntrack_tuple *);
> +	void (*print_tuple)(struct seq_file *s,
> +			    const struct nf_conntrack_tuple *);
>  
>  	/* Print out the private part of the conntrack. */
>  	void (*print_conntrack)(struct seq_file *s, struct nf_conn *);
> diff --git a/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c b/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c
> index a054fe083431..5c61328b7704 100644
> --- a/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c
> +++ b/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c
> @@ -56,11 +56,11 @@ static bool ipv4_invert_tuple(struct nf_conntrack_tuple *tuple,
>  	return true;
>  }
>  
> -static int ipv4_print_tuple(struct seq_file *s,
> +static void ipv4_print_tuple(struct seq_file *s,
>  			    const struct nf_conntrack_tuple *tuple)
>  {
> -	return seq_printf(s, "src=%pI4 dst=%pI4 ",
> -			  &tuple->src.u3.ip, &tuple->dst.u3.ip);
> +	seq_printf(s, "src=%pI4 dst=%pI4 ",
> +		   &tuple->src.u3.ip, &tuple->dst.u3.ip);
>  }
>  
>  static int ipv4_get_l4proto(const struct sk_buff *skb, unsigned int nhoff,
> diff --git a/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4_compat.c b/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4_compat.c
> index 91f207c2cb69..d927f9e72130 100644
> --- a/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4_compat.c
> +++ b/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4_compat.c
> @@ -153,8 +153,10 @@ static int ct_seq_show(struct seq_file *s, void *v)
>  	if (seq_has_overflowed(s))
>  		goto release;
>  
> -	if (print_tuple(s, &ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple,
> -			l3proto, l4proto))
> +	print_tuple(s, &ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple,
> +		    l3proto, l4proto);
> +
> +	if (seq_has_overflowed(s))
>  		goto release;
>  
>  	if (seq_print_acct(s, ct, IP_CT_DIR_ORIGINAL))
> @@ -164,8 +166,10 @@ static int ct_seq_show(struct seq_file *s, void *v)
>  		if (seq_printf(s, "[UNREPLIED] "))
>  			goto release;
>  
> -	if (print_tuple(s, &ct->tuplehash[IP_CT_DIR_REPLY].tuple,
> -			l3proto, l4proto))
> +	print_tuple(s, &ct->tuplehash[IP_CT_DIR_REPLY].tuple,
> +		    l3proto, l4proto);
> +
> +	if (seq_has_overflowed(s))
>  		goto release;
>  
>  	if (seq_print_acct(s, ct, IP_CT_DIR_REPLY))
> diff --git a/net/ipv4/netfilter/nf_conntrack_proto_icmp.c b/net/ipv4/netfilter/nf_conntrack_proto_icmp.c
> index b91b2641adda..80d5554b9a88 100644
> --- a/net/ipv4/netfilter/nf_conntrack_proto_icmp.c
> +++ b/net/ipv4/netfilter/nf_conntrack_proto_icmp.c
> @@ -72,13 +72,13 @@ static bool icmp_invert_tuple(struct nf_conntrack_tuple *tuple,
>  }
>  
>  /* Print out the per-protocol part of the tuple. */
> -static int icmp_print_tuple(struct seq_file *s,
> +static void icmp_print_tuple(struct seq_file *s,
>  			    const struct nf_conntrack_tuple *tuple)
>  {
> -	return seq_printf(s, "type=%u code=%u id=%u ",
> -			  tuple->dst.u.icmp.type,
> -			  tuple->dst.u.icmp.code,
> -			  ntohs(tuple->src.u.icmp.id));
> +	seq_printf(s, "type=%u code=%u id=%u ",
> +		   tuple->dst.u.icmp.type,
> +		   tuple->dst.u.icmp.code,
> +		   ntohs(tuple->src.u.icmp.id));
>  }
>  
>  static unsigned int *icmp_get_timeouts(struct net *net)
> diff --git a/net/ipv6/netfilter/nf_conntrack_l3proto_ipv6.c b/net/ipv6/netfilter/nf_conntrack_l3proto_ipv6.c
> index 4cbc6b290dd5..b68d0e59c1f8 100644
> --- a/net/ipv6/netfilter/nf_conntrack_l3proto_ipv6.c
> +++ b/net/ipv6/netfilter/nf_conntrack_l3proto_ipv6.c
> @@ -60,11 +60,11 @@ static bool ipv6_invert_tuple(struct nf_conntrack_tuple *tuple,
>  	return true;
>  }
>  
> -static int ipv6_print_tuple(struct seq_file *s,
> +static void ipv6_print_tuple(struct seq_file *s,
>  			    const struct nf_conntrack_tuple *tuple)
>  {
> -	return seq_printf(s, "src=%pI6 dst=%pI6 ",
> -			  tuple->src.u3.ip6, tuple->dst.u3.ip6);
> +	seq_printf(s, "src=%pI6 dst=%pI6 ",
> +		   tuple->src.u3.ip6, tuple->dst.u3.ip6);
>  }
>  
>  static int ipv6_get_l4proto(const struct sk_buff *skb, unsigned int nhoff,
> diff --git a/net/ipv6/netfilter/nf_conntrack_proto_icmpv6.c b/net/ipv6/netfilter/nf_conntrack_proto_icmpv6.c
> index b3807c5cb888..90388d606483 100644
> --- a/net/ipv6/netfilter/nf_conntrack_proto_icmpv6.c
> +++ b/net/ipv6/netfilter/nf_conntrack_proto_icmpv6.c
> @@ -84,13 +84,13 @@ static bool icmpv6_invert_tuple(struct nf_conntrack_tuple *tuple,
>  }
>  
>  /* Print out the per-protocol part of the tuple. */
> -static int icmpv6_print_tuple(struct seq_file *s,
> +static void icmpv6_print_tuple(struct seq_file *s,
>  			      const struct nf_conntrack_tuple *tuple)
>  {
> -	return seq_printf(s, "type=%u code=%u id=%u ",
> -			  tuple->dst.u.icmp.type,
> -			  tuple->dst.u.icmp.code,
> -			  ntohs(tuple->src.u.icmp.id));
> +	seq_printf(s, "type=%u code=%u id=%u ",
> +		   tuple->dst.u.icmp.type,
> +		   tuple->dst.u.icmp.code,
> +		   ntohs(tuple->src.u.icmp.id));
>  }
>  
>  static unsigned int *icmpv6_get_timeouts(struct net *net)
> diff --git a/net/netfilter/nf_conntrack_l3proto_generic.c b/net/netfilter/nf_conntrack_l3proto_generic.c
> index e7eb807fe07d..cf9ace70bece 100644
> --- a/net/netfilter/nf_conntrack_l3proto_generic.c
> +++ b/net/netfilter/nf_conntrack_l3proto_generic.c
> @@ -49,10 +49,9 @@ static bool generic_invert_tuple(struct nf_conntrack_tuple *tuple,
>  	return true;
>  }
>  
> -static int generic_print_tuple(struct seq_file *s,
> -			    const struct nf_conntrack_tuple *tuple)
> +static void generic_print_tuple(struct seq_file *s,
> +				const struct nf_conntrack_tuple *tuple)
>  {
> -	return 0;
>  }
>  
>  static int generic_get_l4proto(const struct sk_buff *skb, unsigned int nhoff,
> diff --git a/net/netfilter/nf_conntrack_proto_dccp.c b/net/netfilter/nf_conntrack_proto_dccp.c
> index 15971177470a..6dd995c7c72b 100644
> --- a/net/netfilter/nf_conntrack_proto_dccp.c
> +++ b/net/netfilter/nf_conntrack_proto_dccp.c
> @@ -618,12 +618,12 @@ out_invalid:
>  	return -NF_ACCEPT;
>  }
>  
> -static int dccp_print_tuple(struct seq_file *s,
> -			    const struct nf_conntrack_tuple *tuple)
> +static void dccp_print_tuple(struct seq_file *s,
> +			     const struct nf_conntrack_tuple *tuple)
>  {
> -	return seq_printf(s, "sport=%hu dport=%hu ",
> -			  ntohs(tuple->src.u.dccp.port),
> -			  ntohs(tuple->dst.u.dccp.port));
> +	seq_printf(s, "sport=%hu dport=%hu ",
> +		   ntohs(tuple->src.u.dccp.port),
> +		   ntohs(tuple->dst.u.dccp.port));
>  }
>  
>  static void dccp_print_conntrack(struct seq_file *s, struct nf_conn *ct)
> diff --git a/net/netfilter/nf_conntrack_proto_generic.c b/net/netfilter/nf_conntrack_proto_generic.c
> index 957c1db66652..60865f110309 100644
> --- a/net/netfilter/nf_conntrack_proto_generic.c
> +++ b/net/netfilter/nf_conntrack_proto_generic.c
> @@ -63,10 +63,9 @@ static bool generic_invert_tuple(struct nf_conntrack_tuple *tuple,
>  }
>  
>  /* Print out the per-protocol part of the tuple. */
> -static int generic_print_tuple(struct seq_file *s,
> -			       const struct nf_conntrack_tuple *tuple)
> +static void generic_print_tuple(struct seq_file *s,
> +				const struct nf_conntrack_tuple *tuple)
>  {
> -	return 0;
>  }
>  
>  static unsigned int *generic_get_timeouts(struct net *net)
> diff --git a/net/netfilter/nf_conntrack_proto_gre.c b/net/netfilter/nf_conntrack_proto_gre.c
> index cba607ada069..7648674f29c3 100644
> --- a/net/netfilter/nf_conntrack_proto_gre.c
> +++ b/net/netfilter/nf_conntrack_proto_gre.c
> @@ -226,12 +226,12 @@ static bool gre_pkt_to_tuple(const struct sk_buff *skb, unsigned int dataoff,
>  }
>  
>  /* print gre part of tuple */
> -static int gre_print_tuple(struct seq_file *s,
> -			   const struct nf_conntrack_tuple *tuple)
> +static void gre_print_tuple(struct seq_file *s,
> +			    const struct nf_conntrack_tuple *tuple)
>  {
> -	return seq_printf(s, "srckey=0x%x dstkey=0x%x ",
> -			  ntohs(tuple->src.u.gre.key),
> -			  ntohs(tuple->dst.u.gre.key));
> +	seq_printf(s, "srckey=0x%x dstkey=0x%x ",
> +		   ntohs(tuple->src.u.gre.key),
> +		   ntohs(tuple->dst.u.gre.key));
>  }
>  
>  /* print private data for conntrack */
> diff --git a/net/netfilter/nf_conntrack_proto_sctp.c b/net/netfilter/nf_conntrack_proto_sctp.c
> index c61f4cd6407d..b45da90fad32 100644
> --- a/net/netfilter/nf_conntrack_proto_sctp.c
> +++ b/net/netfilter/nf_conntrack_proto_sctp.c
> @@ -166,12 +166,12 @@ static bool sctp_invert_tuple(struct nf_conntrack_tuple *tuple,
>  }
>  
>  /* Print out the per-protocol part of the tuple. */
> -static int sctp_print_tuple(struct seq_file *s,
> -			    const struct nf_conntrack_tuple *tuple)
> +static void sctp_print_tuple(struct seq_file *s,
> +			     const struct nf_conntrack_tuple *tuple)
>  {
> -	return seq_printf(s, "sport=%hu dport=%hu ",
> -			  ntohs(tuple->src.u.sctp.port),
> -			  ntohs(tuple->dst.u.sctp.port));
> +	seq_printf(s, "sport=%hu dport=%hu ",
> +		   ntohs(tuple->src.u.sctp.port),
> +		   ntohs(tuple->dst.u.sctp.port));
>  }
>  
>  /* Print out the private part of the conntrack. */
> diff --git a/net/netfilter/nf_conntrack_proto_tcp.c b/net/netfilter/nf_conntrack_proto_tcp.c
> index 79668fd3db96..36a3ac8ee9f5 100644
> --- a/net/netfilter/nf_conntrack_proto_tcp.c
> +++ b/net/netfilter/nf_conntrack_proto_tcp.c
> @@ -302,12 +302,12 @@ static bool tcp_invert_tuple(struct nf_conntrack_tuple *tuple,
>  }
>  
>  /* Print out the per-protocol part of the tuple. */
> -static int tcp_print_tuple(struct seq_file *s,
> -			   const struct nf_conntrack_tuple *tuple)
> +static void tcp_print_tuple(struct seq_file *s,
> +			    const struct nf_conntrack_tuple *tuple)
>  {
> -	return seq_printf(s, "sport=%hu dport=%hu ",
> -			  ntohs(tuple->src.u.tcp.port),
> -			  ntohs(tuple->dst.u.tcp.port));
> +	seq_printf(s, "sport=%hu dport=%hu ",
> +		   ntohs(tuple->src.u.tcp.port),
> +		   ntohs(tuple->dst.u.tcp.port));
>  }
>  
>  /* Print out the private part of the conntrack. */
> diff --git a/net/netfilter/nf_conntrack_proto_udp.c b/net/netfilter/nf_conntrack_proto_udp.c
> index 9d7721cbce4b..6957281ffee5 100644
> --- a/net/netfilter/nf_conntrack_proto_udp.c
> +++ b/net/netfilter/nf_conntrack_proto_udp.c
> @@ -63,12 +63,12 @@ static bool udp_invert_tuple(struct nf_conntrack_tuple *tuple,
>  }
>  
>  /* Print out the per-protocol part of the tuple. */
> -static int udp_print_tuple(struct seq_file *s,
> -			   const struct nf_conntrack_tuple *tuple)
> +static void udp_print_tuple(struct seq_file *s,
> +			    const struct nf_conntrack_tuple *tuple)
>  {
> -	return seq_printf(s, "sport=%hu dport=%hu ",
> -			  ntohs(tuple->src.u.udp.port),
> -			  ntohs(tuple->dst.u.udp.port));
> +	seq_printf(s, "sport=%hu dport=%hu ",
> +		   ntohs(tuple->src.u.udp.port),
> +		   ntohs(tuple->dst.u.udp.port));
>  }
>  
>  static unsigned int *udp_get_timeouts(struct net *net)
> diff --git a/net/netfilter/nf_conntrack_proto_udplite.c b/net/netfilter/nf_conntrack_proto_udplite.c
> index 2750e6c69f82..c5903d1649f9 100644
> --- a/net/netfilter/nf_conntrack_proto_udplite.c
> +++ b/net/netfilter/nf_conntrack_proto_udplite.c
> @@ -71,12 +71,12 @@ static bool udplite_invert_tuple(struct nf_conntrack_tuple *tuple,
>  }
>  
>  /* Print out the per-protocol part of the tuple. */
> -static int udplite_print_tuple(struct seq_file *s,
> -			       const struct nf_conntrack_tuple *tuple)
> +static void udplite_print_tuple(struct seq_file *s,
> +				const struct nf_conntrack_tuple *tuple)
>  {
> -	return seq_printf(s, "sport=%hu dport=%hu ",
> -			  ntohs(tuple->src.u.udp.port),
> -			  ntohs(tuple->dst.u.udp.port));
> +	seq_printf(s, "sport=%hu dport=%hu ",
> +		   ntohs(tuple->src.u.udp.port),
> +		   ntohs(tuple->dst.u.udp.port));
>  }
>  
>  static unsigned int *udplite_get_timeouts(struct net *net)
> diff --git a/net/netfilter/nf_conntrack_standalone.c b/net/netfilter/nf_conntrack_standalone.c
> index 348aa3602787..23a0dcab21d4 100644
> --- a/net/netfilter/nf_conntrack_standalone.c
> +++ b/net/netfilter/nf_conntrack_standalone.c
> @@ -36,12 +36,13 @@
>  MODULE_LICENSE("GPL");
>  
>  #ifdef CONFIG_NF_CONNTRACK_PROCFS
> -int
> +void
>  print_tuple(struct seq_file *s, const struct nf_conntrack_tuple *tuple,
>              const struct nf_conntrack_l3proto *l3proto,
>              const struct nf_conntrack_l4proto *l4proto)
>  {
> -	return l3proto->print_tuple(s, tuple) || l4proto->print_tuple(s, tuple);
> +	l3proto->print_tuple(s, tuple);
> +	l4proto->print_tuple(s, tuple);
>  }
>  EXPORT_SYMBOL_GPL(print_tuple);
>  
> @@ -202,9 +203,8 @@ static int ct_seq_show(struct seq_file *s, void *v)
>  	if (l4proto->print_conntrack)
>  		l4proto->print_conntrack(s, ct);
>  
> -	if (print_tuple(s, &ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple,
> -			l3proto, l4proto))
> -		goto release;
> +	print_tuple(s, &ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple,
> +		    l3proto, l4proto);
>  
>  	if (seq_print_acct(s, ct, IP_CT_DIR_ORIGINAL))
>  		goto release;
> @@ -213,9 +213,8 @@ static int ct_seq_show(struct seq_file *s, void *v)
>  		if (seq_printf(s, "[UNREPLIED] "))
>  			goto release;
>  
> -	if (print_tuple(s, &ct->tuplehash[IP_CT_DIR_REPLY].tuple,
> -			l3proto, l4proto))
> -		goto release;
> +	print_tuple(s, &ct->tuplehash[IP_CT_DIR_REPLY].tuple,
> +		    l3proto, l4proto);
>  
>  	if (seq_print_acct(s, ct, IP_CT_DIR_REPLY))
>  		goto release;

[RFA][PATCH 4/8] netfilter: Remove checks of seq_printf() return values

[Steven Rostedt]

[-- Attachment #1: 0004-netfilter-Remove-checks-of-seq_printf-return-values.patch --]
[-- Type: text/plain, Size: 13276 bytes --]

From: "Steven Rostedt (Red Hat)" <rostedt@goodmis.org>

[ REQUEST FOR ACKS ]

The return value of seq_printf() is soon to be removed. Remove the
checks from seq_printf() in favor of seq_has_overflowed().

Cc: Pablo Neira Ayuso <pablo@netfilter.org>
Cc: Patrick McHardy <kaber@trash.net>
Cc: Jozsef Kadlecsik <kadlec@blackhole.kfki.hu>
Cc: netfilter-devel@vger.kernel.org
Cc: coreteam@netfilter.org
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 .../netfilter/nf_conntrack_l3proto_ipv4_compat.c   | 36 ++++++-------
 net/netfilter/nf_conntrack_standalone.c            | 60 +++++++++++-----------
 net/netfilter/nf_log.c                             | 30 ++++++-----
 net/netfilter/nfnetlink_queue_core.c               | 13 ++---
 net/netfilter/x_tables.c                           | 19 ++++---
 net/netfilter/xt_hashlimit.c                       | 36 ++++++-------
 6 files changed, 97 insertions(+), 97 deletions(-)

diff --git a/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4_compat.c b/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4_compat.c
index d927f9e72130..a460a87e14f8 100644
--- a/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4_compat.c
+++ b/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4_compat.c
@@ -94,7 +94,7 @@ static void ct_seq_stop(struct seq_file *s, void *v)
 }
 
 #ifdef CONFIG_NF_CONNTRACK_SECMARK
-static int ct_show_secctx(struct seq_file *s, const struct nf_conn *ct)
+static void ct_show_secctx(struct seq_file *s, const struct nf_conn *ct)
 {
 	int ret;
 	u32 len;
@@ -102,17 +102,15 @@ static int ct_show_secctx(struct seq_file *s, const struct nf_conn *ct)
 
 	ret = security_secid_to_secctx(ct->secmark, &secctx, &len);
 	if (ret)
-		return 0;
+		return;
 
-	ret = seq_printf(s, "secctx=%s ", secctx);
+	seq_printf(s, "secctx=%s ", secctx);
 
 	security_release_secctx(secctx, len);
-	return ret;
 }
 #else
-static inline int ct_show_secctx(struct seq_file *s, const struct nf_conn *ct)
+static inline void ct_show_secctx(struct seq_file *s, const struct nf_conn *ct)
 {
-	return 0;
 }
 #endif
 
@@ -141,11 +139,10 @@ static int ct_seq_show(struct seq_file *s, void *v)
 	NF_CT_ASSERT(l4proto);
 
 	ret = -ENOSPC;
-	if (seq_printf(s, "%-8s %u %ld ",
-		      l4proto->name, nf_ct_protonum(ct),
-		      timer_pending(&ct->timeout)
-		      ? (long)(ct->timeout.expires - jiffies)/HZ : 0) != 0)
-		goto release;
+	seq_printf(s, "%-8s %u %ld ",
+		   l4proto->name, nf_ct_protonum(ct),
+		   timer_pending(&ct->timeout)
+		   ? (long)(ct->timeout.expires - jiffies)/HZ : 0);
 
 	if (l4proto->print_conntrack)
 		l4proto->print_conntrack(s, ct);
@@ -163,8 +160,7 @@ static int ct_seq_show(struct seq_file *s, void *v)
 		goto release;
 
 	if (!(test_bit(IPS_SEEN_REPLY_BIT, &ct->status)))
-		if (seq_printf(s, "[UNREPLIED] "))
-			goto release;
+		seq_printf(s, "[UNREPLIED] ");
 
 	print_tuple(s, &ct->tuplehash[IP_CT_DIR_REPLY].tuple,
 		    l3proto, l4proto);
@@ -176,19 +172,19 @@ static int ct_seq_show(struct seq_file *s, void *v)
 		goto release;
 
 	if (test_bit(IPS_ASSURED_BIT, &ct->status))
-		if (seq_printf(s, "[ASSURED] "))
-			goto release;
+		seq_printf(s, "[ASSURED] ");
 
 #ifdef CONFIG_NF_CONNTRACK_MARK
-	if (seq_printf(s, "mark=%u ", ct->mark))
-		goto release;
+	seq_printf(s, "mark=%u ", ct->mark);
 #endif
 
-	if (ct_show_secctx(s, ct))
-		goto release;
+	ct_show_secctx(s, ct);
+
+	seq_printf(s, "use=%u\n", atomic_read(&ct->ct_general.use));
 
-	if (seq_printf(s, "use=%u\n", atomic_read(&ct->ct_general.use)))
+	if (seq_has_overflowed(s))
 		goto release;
+
 	ret = 0;
 release:
 	nf_ct_put(ct);
diff --git a/net/netfilter/nf_conntrack_standalone.c b/net/netfilter/nf_conntrack_standalone.c
index 23a0dcab21d4..fc823fa5dcf5 100644
--- a/net/netfilter/nf_conntrack_standalone.c
+++ b/net/netfilter/nf_conntrack_standalone.c
@@ -120,7 +120,7 @@ static void ct_seq_stop(struct seq_file *s, void *v)
 }
 
 #ifdef CONFIG_NF_CONNTRACK_SECMARK
-static int ct_show_secctx(struct seq_file *s, const struct nf_conn *ct)
+static void ct_show_secctx(struct seq_file *s, const struct nf_conn *ct)
 {
 	int ret;
 	u32 len;
@@ -128,22 +128,20 @@ static int ct_show_secctx(struct seq_file *s, const struct nf_conn *ct)
 
 	ret = security_secid_to_secctx(ct->secmark, &secctx, &len);
 	if (ret)
-		return 0;
+		return;
 
-	ret = seq_printf(s, "secctx=%s ", secctx);
+	seq_printf(s, "secctx=%s ", secctx);
 
 	security_release_secctx(secctx, len);
-	return ret;
 }
 #else
-static inline int ct_show_secctx(struct seq_file *s, const struct nf_conn *ct)
+static inline void ct_show_secctx(struct seq_file *s, const struct nf_conn *ct)
 {
-	return 0;
 }
 #endif
 
 #ifdef CONFIG_NF_CONNTRACK_TIMESTAMP
-static int ct_show_delta_time(struct seq_file *s, const struct nf_conn *ct)
+static void ct_show_delta_time(struct seq_file *s, const struct nf_conn *ct)
 {
 	struct ct_iter_state *st = s->private;
 	struct nf_conn_tstamp *tstamp;
@@ -157,16 +155,15 @@ static int ct_show_delta_time(struct seq_file *s, const struct nf_conn *ct)
 		else
 			delta_time = 0;
 
-		return seq_printf(s, "delta-time=%llu ",
-				  (unsigned long long)delta_time);
+		seq_printf(s, "delta-time=%llu ",
+			   (unsigned long long)delta_time);
 	}
-	return 0;
+	return;
 }
 #else
-static inline int
+static inline void
 ct_show_delta_time(struct seq_file *s, const struct nf_conn *ct)
 {
-	return 0;
 }
 #endif
 
@@ -193,12 +190,11 @@ static int ct_seq_show(struct seq_file *s, void *v)
 	NF_CT_ASSERT(l4proto);
 
 	ret = -ENOSPC;
-	if (seq_printf(s, "%-8s %u %-8s %u %ld ",
-		       l3proto->name, nf_ct_l3num(ct),
-		       l4proto->name, nf_ct_protonum(ct),
-		       timer_pending(&ct->timeout)
-		       ? (long)(ct->timeout.expires - jiffies)/HZ : 0) != 0)
-		goto release;
+	seq_printf(s, "%-8s %u %-8s %u %ld ",
+		   l3proto->name, nf_ct_l3num(ct),
+		   l4proto->name, nf_ct_protonum(ct),
+		   timer_pending(&ct->timeout)
+		   ? (long)(ct->timeout.expires - jiffies)/HZ : 0);
 
 	if (l4proto->print_conntrack)
 		l4proto->print_conntrack(s, ct);
@@ -206,12 +202,14 @@ static int ct_seq_show(struct seq_file *s, void *v)
 	print_tuple(s, &ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple,
 		    l3proto, l4proto);
 
+	if (seq_has_overflowed(s))
+		goto release;
+
 	if (seq_print_acct(s, ct, IP_CT_DIR_ORIGINAL))
 		goto release;
 
 	if (!(test_bit(IPS_SEEN_REPLY_BIT, &ct->status)))
-		if (seq_printf(s, "[UNREPLIED] "))
-			goto release;
+		seq_printf(s, "[UNREPLIED] ");
 
 	print_tuple(s, &ct->tuplehash[IP_CT_DIR_REPLY].tuple,
 		    l3proto, l4proto);
@@ -220,26 +218,26 @@ static int ct_seq_show(struct seq_file *s, void *v)
 		goto release;
 
 	if (test_bit(IPS_ASSURED_BIT, &ct->status))
-		if (seq_printf(s, "[ASSURED] "))
-			goto release;
+		seq_printf(s, "[ASSURED] ");
 
-#if defined(CONFIG_NF_CONNTRACK_MARK)
-	if (seq_printf(s, "mark=%u ", ct->mark))
+	if (seq_has_overflowed(s))
 		goto release;
+
+#if defined(CONFIG_NF_CONNTRACK_MARK)
+	seq_printf(s, "mark=%u ", ct->mark);
 #endif
 
-	if (ct_show_secctx(s, ct))
-		goto release;
+	ct_show_secctx(s, ct);
 
 #ifdef CONFIG_NF_CONNTRACK_ZONES
-	if (seq_printf(s, "zone=%u ", nf_ct_zone(ct)))
-		goto release;
+	seq_printf(s, "zone=%u ", nf_ct_zone(ct));
 #endif
 
-	if (ct_show_delta_time(s, ct))
-		goto release;
+	ct_show_delta_time(s, ct);
+
+	seq_printf(s, "use=%u\n", atomic_read(&ct->ct_general.use));
 
-	if (seq_printf(s, "use=%u\n", atomic_read(&ct->ct_general.use)))
+	if (seq_has_overflowed(s))
 		goto release;
 
 	ret = 0;
diff --git a/net/netfilter/nf_log.c b/net/netfilter/nf_log.c
index d7197649dba6..6e3b9117db1f 100644
--- a/net/netfilter/nf_log.c
+++ b/net/netfilter/nf_log.c
@@ -294,19 +294,19 @@ static int seq_show(struct seq_file *s, void *v)
 {
 	loff_t *pos = v;
 	const struct nf_logger *logger;
-	int i, ret;
+	int i;
 	struct net *net = seq_file_net(s);
 
 	logger = rcu_dereference_protected(net->nf.nf_loggers[*pos],
 					   lockdep_is_held(&nf_log_mutex));
 
 	if (!logger)
-		ret = seq_printf(s, "%2lld NONE (", *pos);
+		seq_printf(s, "%2lld NONE (", *pos);
 	else
-		ret = seq_printf(s, "%2lld %s (", *pos, logger->name);
+		seq_printf(s, "%2lld %s (", *pos, logger->name);
 
-	if (ret < 0)
-		return ret;
+	if (seq_has_overflowed(s))
+		return -ENOSPC;
 
 	for (i = 0; i < NF_LOG_TYPE_MAX; i++) {
 		if (loggers[*pos][i] == NULL)
@@ -314,17 +314,19 @@ static int seq_show(struct seq_file *s, void *v)
 
 		logger = rcu_dereference_protected(loggers[*pos][i],
 					   lockdep_is_held(&nf_log_mutex));
-		ret = seq_printf(s, "%s", logger->name);
-		if (ret < 0)
-			return ret;
-		if (i == 0 && loggers[*pos][i + 1] != NULL) {
-			ret = seq_printf(s, ",");
-			if (ret < 0)
-				return ret;
-		}
+		seq_printf(s, "%s", logger->name);
+		if (i == 0 && loggers[*pos][i + 1] != NULL)
+			seq_printf(s, ",");
+
+		if (seq_has_overflowed(s))
+			return -ENOSPC;
 	}
 
-	return seq_printf(s, ")\n");
+	seq_printf(s, ")\n");
+
+	if (seq_has_overflowed(s))
+		return -ENOSPC;
+	return 0;
 }
 
 static const struct seq_operations nflog_seq_ops = {
diff --git a/net/netfilter/nfnetlink_queue_core.c b/net/netfilter/nfnetlink_queue_core.c
index a82077d9f59b..f823f1538c4f 100644
--- a/net/netfilter/nfnetlink_queue_core.c
+++ b/net/netfilter/nfnetlink_queue_core.c
@@ -1242,12 +1242,13 @@ static int seq_show(struct seq_file *s, void *v)
 {
 	const struct nfqnl_instance *inst = v;
 
-	return seq_printf(s, "%5d %6d %5d %1d %5d %5d %5d %8d %2d\n",
-			  inst->queue_num,
-			  inst->peer_portid, inst->queue_total,
-			  inst->copy_mode, inst->copy_range,
-			  inst->queue_dropped, inst->queue_user_dropped,
-			  inst->id_sequence, 1);
+	seq_printf(s, "%5d %6d %5d %1d %5d %5d %5d %8d %2d\n",
+		   inst->queue_num,
+		   inst->peer_portid, inst->queue_total,
+		   inst->copy_mode, inst->copy_range,
+		   inst->queue_dropped, inst->queue_user_dropped,
+		   inst->id_sequence, 1);
+	return seq_has_overflowed(s);
 }
 
 static const struct seq_operations nfqnl_seq_ops = {
diff --git a/net/netfilter/x_tables.c b/net/netfilter/x_tables.c
index 133eb4772f12..51a459c3c649 100644
--- a/net/netfilter/x_tables.c
+++ b/net/netfilter/x_tables.c
@@ -947,9 +947,10 @@ static int xt_table_seq_show(struct seq_file *seq, void *v)
 {
 	struct xt_table *table = list_entry(v, struct xt_table, list);
 
-	if (strlen(table->name))
-		return seq_printf(seq, "%s\n", table->name);
-	else
+	if (strlen(table->name)) {
+		seq_printf(seq, "%s\n", table->name);
+		return seq_has_overflowed(seq);
+	} else
 		return 0;
 }
 
@@ -1086,8 +1087,10 @@ static int xt_match_seq_show(struct seq_file *seq, void *v)
 		if (trav->curr == trav->head)
 			return 0;
 		match = list_entry(trav->curr, struct xt_match, list);
-		return (*match->name == '\0') ? 0 :
-		       seq_printf(seq, "%s\n", match->name);
+		if (*match->name == '\0')
+			return 0;
+		seq_printf(seq, "%s\n", match->name);
+		return seq_has_overflowed(seq);
 	}
 	return 0;
 }
@@ -1139,8 +1142,10 @@ static int xt_target_seq_show(struct seq_file *seq, void *v)
 		if (trav->curr == trav->head)
 			return 0;
 		target = list_entry(trav->curr, struct xt_target, list);
-		return (*target->name == '\0') ? 0 :
-		       seq_printf(seq, "%s\n", target->name);
+		if (*target->name == '\0')
+			return 0;
+		seq_printf(seq, "%s\n", target->name);
+		return seq_has_overflowed(seq);
 	}
 	return 0;
 }
diff --git a/net/netfilter/xt_hashlimit.c b/net/netfilter/xt_hashlimit.c
index 05fbc2a0be46..178696852bde 100644
--- a/net/netfilter/xt_hashlimit.c
+++ b/net/netfilter/xt_hashlimit.c
@@ -789,7 +789,6 @@ static void dl_seq_stop(struct seq_file *s, void *v)
 static int dl_seq_real_show(struct dsthash_ent *ent, u_int8_t family,
 				   struct seq_file *s)
 {
-	int res;
 	const struct xt_hashlimit_htable *ht = s->private;
 
 	spin_lock(&ent->lock);
@@ -798,33 +797,32 @@ static int dl_seq_real_show(struct dsthash_ent *ent, u_int8_t family,
 
 	switch (family) {
 	case NFPROTO_IPV4:
-		res = seq_printf(s, "%ld %pI4:%u->%pI4:%u %u %u %u\n",
-				 (long)(ent->expires - jiffies)/HZ,
-				 &ent->dst.ip.src,
-				 ntohs(ent->dst.src_port),
-				 &ent->dst.ip.dst,
-				 ntohs(ent->dst.dst_port),
-				 ent->rateinfo.credit, ent->rateinfo.credit_cap,
-				 ent->rateinfo.cost);
+		seq_printf(s, "%ld %pI4:%u->%pI4:%u %u %u %u\n",
+			   (long)(ent->expires - jiffies)/HZ,
+			   &ent->dst.ip.src,
+			   ntohs(ent->dst.src_port),
+			   &ent->dst.ip.dst,
+			   ntohs(ent->dst.dst_port),
+			   ent->rateinfo.credit, ent->rateinfo.credit_cap,
+			   ent->rateinfo.cost);
 		break;
 #if IS_ENABLED(CONFIG_IP6_NF_IPTABLES)
 	case NFPROTO_IPV6:
-		res = seq_printf(s, "%ld %pI6:%u->%pI6:%u %u %u %u\n",
-				 (long)(ent->expires - jiffies)/HZ,
-				 &ent->dst.ip6.src,
-				 ntohs(ent->dst.src_port),
-				 &ent->dst.ip6.dst,
-				 ntohs(ent->dst.dst_port),
-				 ent->rateinfo.credit, ent->rateinfo.credit_cap,
-				 ent->rateinfo.cost);
+		seq_printf(s, "%ld %pI6:%u->%pI6:%u %u %u %u\n",
+			   (long)(ent->expires - jiffies)/HZ,
+			   &ent->dst.ip6.src,
+			   ntohs(ent->dst.src_port),
+			   &ent->dst.ip6.dst,
+			   ntohs(ent->dst.dst_port),
+			   ent->rateinfo.credit, ent->rateinfo.credit_cap,
+			   ent->rateinfo.cost);
 		break;
 #endif
 	default:
 		BUG();
-		res = 0;
 	}
 	spin_unlock(&ent->lock);
-	return res;
+	return seq_has_overflowed(s);
 }
 
 static int dl_seq_show(struct seq_file *s, void *v)
-- 
2.1.1

Re: [RFA][PATCH 4/8] netfilter: Remove checks of seq_printf() return values

[Steven Rostedt]

On Wed, 29 Oct 2014 17:56:06 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

> From: "Steven Rostedt (Red Hat)" <rostedt@goodmis.org>
> 
> [ REQUEST FOR ACKS ]

Can any of the netfilter maintainers give me an Acked-by for this?

Thanks!

-- Steve

> 
> The return value of seq_printf() is soon to be removed. Remove the
> checks from seq_printf() in favor of seq_has_overflowed().
> 
> Cc: Pablo Neira Ayuso <pablo@netfilter.org>
> Cc: Patrick McHardy <kaber@trash.net>
> Cc: Jozsef Kadlecsik <kadlec@blackhole.kfki.hu>
> Cc: netfilter-devel@vger.kernel.org
> Cc: coreteam@netfilter.org
> Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
> ---
>  .../netfilter/nf_conntrack_l3proto_ipv4_compat.c   | 36 ++++++-------
>  net/netfilter/nf_conntrack_standalone.c            | 60 +++++++++++-----------
>  net/netfilter/nf_log.c                             | 30 ++++++-----
>  net/netfilter/nfnetlink_queue_core.c               | 13 ++---
>  net/netfilter/x_tables.c                           | 19 ++++---
>  net/netfilter/xt_hashlimit.c                       | 36 ++++++-------
>  6 files changed, 97 insertions(+), 97 deletions(-)
> 
> diff --git a/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4_compat.c b/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4_compat.c
> index d927f9e72130..a460a87e14f8 100644
> --- a/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4_compat.c
> +++ b/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4_compat.c
> @@ -94,7 +94,7 @@ static void ct_seq_stop(struct seq_file *s, void *v)
>  }
>  
>  #ifdef CONFIG_NF_CONNTRACK_SECMARK
> -static int ct_show_secctx(struct seq_file *s, const struct nf_conn *ct)
> +static void ct_show_secctx(struct seq_file *s, const struct nf_conn *ct)
>  {
>  	int ret;
>  	u32 len;
> @@ -102,17 +102,15 @@ static int ct_show_secctx(struct seq_file *s, const struct nf_conn *ct)
>  
>  	ret = security_secid_to_secctx(ct->secmark, &secctx, &len);
>  	if (ret)
> -		return 0;
> +		return;
>  
> -	ret = seq_printf(s, "secctx=%s ", secctx);
> +	seq_printf(s, "secctx=%s ", secctx);
>  
>  	security_release_secctx(secctx, len);
> -	return ret;
>  }
>  #else
> -static inline int ct_show_secctx(struct seq_file *s, const struct nf_conn *ct)
> +static inline void ct_show_secctx(struct seq_file *s, const struct nf_conn *ct)
>  {
> -	return 0;
>  }
>  #endif
>  
> @@ -141,11 +139,10 @@ static int ct_seq_show(struct seq_file *s, void *v)
>  	NF_CT_ASSERT(l4proto);
>  
>  	ret = -ENOSPC;
> -	if (seq_printf(s, "%-8s %u %ld ",
> -		      l4proto->name, nf_ct_protonum(ct),
> -		      timer_pending(&ct->timeout)
> -		      ? (long)(ct->timeout.expires - jiffies)/HZ : 0) != 0)
> -		goto release;
> +	seq_printf(s, "%-8s %u %ld ",
> +		   l4proto->name, nf_ct_protonum(ct),
> +		   timer_pending(&ct->timeout)
> +		   ? (long)(ct->timeout.expires - jiffies)/HZ : 0);
>  
>  	if (l4proto->print_conntrack)
>  		l4proto->print_conntrack(s, ct);
> @@ -163,8 +160,7 @@ static int ct_seq_show(struct seq_file *s, void *v)
>  		goto release;
>  
>  	if (!(test_bit(IPS_SEEN_REPLY_BIT, &ct->status)))
> -		if (seq_printf(s, "[UNREPLIED] "))
> -			goto release;
> +		seq_printf(s, "[UNREPLIED] ");
>  
>  	print_tuple(s, &ct->tuplehash[IP_CT_DIR_REPLY].tuple,
>  		    l3proto, l4proto);
> @@ -176,19 +172,19 @@ static int ct_seq_show(struct seq_file *s, void *v)
>  		goto release;
>  
>  	if (test_bit(IPS_ASSURED_BIT, &ct->status))
> -		if (seq_printf(s, "[ASSURED] "))
> -			goto release;
> +		seq_printf(s, "[ASSURED] ");
>  
>  #ifdef CONFIG_NF_CONNTRACK_MARK
> -	if (seq_printf(s, "mark=%u ", ct->mark))
> -		goto release;
> +	seq_printf(s, "mark=%u ", ct->mark);
>  #endif
>  
> -	if (ct_show_secctx(s, ct))
> -		goto release;
> +	ct_show_secctx(s, ct);
> +
> +	seq_printf(s, "use=%u\n", atomic_read(&ct->ct_general.use));
>  
> -	if (seq_printf(s, "use=%u\n", atomic_read(&ct->ct_general.use)))
> +	if (seq_has_overflowed(s))
>  		goto release;
> +
>  	ret = 0;
>  release:
>  	nf_ct_put(ct);
> diff --git a/net/netfilter/nf_conntrack_standalone.c b/net/netfilter/nf_conntrack_standalone.c
> index 23a0dcab21d4..fc823fa5dcf5 100644
> --- a/net/netfilter/nf_conntrack_standalone.c
> +++ b/net/netfilter/nf_conntrack_standalone.c
> @@ -120,7 +120,7 @@ static void ct_seq_stop(struct seq_file *s, void *v)
>  }
>  
>  #ifdef CONFIG_NF_CONNTRACK_SECMARK
> -static int ct_show_secctx(struct seq_file *s, const struct nf_conn *ct)
> +static void ct_show_secctx(struct seq_file *s, const struct nf_conn *ct)
>  {
>  	int ret;
>  	u32 len;
> @@ -128,22 +128,20 @@ static int ct_show_secctx(struct seq_file *s, const struct nf_conn *ct)
>  
>  	ret = security_secid_to_secctx(ct->secmark, &secctx, &len);
>  	if (ret)
> -		return 0;
> +		return;
>  
> -	ret = seq_printf(s, "secctx=%s ", secctx);
> +	seq_printf(s, "secctx=%s ", secctx);
>  
>  	security_release_secctx(secctx, len);
> -	return ret;
>  }
>  #else
> -static inline int ct_show_secctx(struct seq_file *s, const struct nf_conn *ct)
> +static inline void ct_show_secctx(struct seq_file *s, const struct nf_conn *ct)
>  {
> -	return 0;
>  }
>  #endif
>  
>  #ifdef CONFIG_NF_CONNTRACK_TIMESTAMP
> -static int ct_show_delta_time(struct seq_file *s, const struct nf_conn *ct)
> +static void ct_show_delta_time(struct seq_file *s, const struct nf_conn *ct)
>  {
>  	struct ct_iter_state *st = s->private;
>  	struct nf_conn_tstamp *tstamp;
> @@ -157,16 +155,15 @@ static int ct_show_delta_time(struct seq_file *s, const struct nf_conn *ct)
>  		else
>  			delta_time = 0;
>  
> -		return seq_printf(s, "delta-time=%llu ",
> -				  (unsigned long long)delta_time);
> +		seq_printf(s, "delta-time=%llu ",
> +			   (unsigned long long)delta_time);
>  	}
> -	return 0;
> +	return;
>  }
>  #else
> -static inline int
> +static inline void
>  ct_show_delta_time(struct seq_file *s, const struct nf_conn *ct)
>  {
> -	return 0;
>  }
>  #endif
>  
> @@ -193,12 +190,11 @@ static int ct_seq_show(struct seq_file *s, void *v)
>  	NF_CT_ASSERT(l4proto);
>  
>  	ret = -ENOSPC;
> -	if (seq_printf(s, "%-8s %u %-8s %u %ld ",
> -		       l3proto->name, nf_ct_l3num(ct),
> -		       l4proto->name, nf_ct_protonum(ct),
> -		       timer_pending(&ct->timeout)
> -		       ? (long)(ct->timeout.expires - jiffies)/HZ : 0) != 0)
> -		goto release;
> +	seq_printf(s, "%-8s %u %-8s %u %ld ",
> +		   l3proto->name, nf_ct_l3num(ct),
> +		   l4proto->name, nf_ct_protonum(ct),
> +		   timer_pending(&ct->timeout)
> +		   ? (long)(ct->timeout.expires - jiffies)/HZ : 0);
>  
>  	if (l4proto->print_conntrack)
>  		l4proto->print_conntrack(s, ct);
> @@ -206,12 +202,14 @@ static int ct_seq_show(struct seq_file *s, void *v)
>  	print_tuple(s, &ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple,
>  		    l3proto, l4proto);
>  
> +	if (seq_has_overflowed(s))
> +		goto release;
> +
>  	if (seq_print_acct(s, ct, IP_CT_DIR_ORIGINAL))
>  		goto release;
>  
>  	if (!(test_bit(IPS_SEEN_REPLY_BIT, &ct->status)))
> -		if (seq_printf(s, "[UNREPLIED] "))
> -			goto release;
> +		seq_printf(s, "[UNREPLIED] ");
>  
>  	print_tuple(s, &ct->tuplehash[IP_CT_DIR_REPLY].tuple,
>  		    l3proto, l4proto);
> @@ -220,26 +218,26 @@ static int ct_seq_show(struct seq_file *s, void *v)
>  		goto release;
>  
>  	if (test_bit(IPS_ASSURED_BIT, &ct->status))
> -		if (seq_printf(s, "[ASSURED] "))
> -			goto release;
> +		seq_printf(s, "[ASSURED] ");
>  
> -#if defined(CONFIG_NF_CONNTRACK_MARK)
> -	if (seq_printf(s, "mark=%u ", ct->mark))
> +	if (seq_has_overflowed(s))
>  		goto release;
> +
> +#if defined(CONFIG_NF_CONNTRACK_MARK)
> +	seq_printf(s, "mark=%u ", ct->mark);
>  #endif
>  
> -	if (ct_show_secctx(s, ct))
> -		goto release;
> +	ct_show_secctx(s, ct);
>  
>  #ifdef CONFIG_NF_CONNTRACK_ZONES
> -	if (seq_printf(s, "zone=%u ", nf_ct_zone(ct)))
> -		goto release;
> +	seq_printf(s, "zone=%u ", nf_ct_zone(ct));
>  #endif
>  
> -	if (ct_show_delta_time(s, ct))
> -		goto release;
> +	ct_show_delta_time(s, ct);
> +
> +	seq_printf(s, "use=%u\n", atomic_read(&ct->ct_general.use));
>  
> -	if (seq_printf(s, "use=%u\n", atomic_read(&ct->ct_general.use)))
> +	if (seq_has_overflowed(s))
>  		goto release;
>  
>  	ret = 0;
> diff --git a/net/netfilter/nf_log.c b/net/netfilter/nf_log.c
> index d7197649dba6..6e3b9117db1f 100644
> --- a/net/netfilter/nf_log.c
> +++ b/net/netfilter/nf_log.c
> @@ -294,19 +294,19 @@ static int seq_show(struct seq_file *s, void *v)
>  {
>  	loff_t *pos = v;
>  	const struct nf_logger *logger;
> -	int i, ret;
> +	int i;
>  	struct net *net = seq_file_net(s);
>  
>  	logger = rcu_dereference_protected(net->nf.nf_loggers[*pos],
>  					   lockdep_is_held(&nf_log_mutex));
>  
>  	if (!logger)
> -		ret = seq_printf(s, "%2lld NONE (", *pos);
> +		seq_printf(s, "%2lld NONE (", *pos);
>  	else
> -		ret = seq_printf(s, "%2lld %s (", *pos, logger->name);
> +		seq_printf(s, "%2lld %s (", *pos, logger->name);
>  
> -	if (ret < 0)
> -		return ret;
> +	if (seq_has_overflowed(s))
> +		return -ENOSPC;
>  
>  	for (i = 0; i < NF_LOG_TYPE_MAX; i++) {
>  		if (loggers[*pos][i] == NULL)
> @@ -314,17 +314,19 @@ static int seq_show(struct seq_file *s, void *v)
>  
>  		logger = rcu_dereference_protected(loggers[*pos][i],
>  					   lockdep_is_held(&nf_log_mutex));
> -		ret = seq_printf(s, "%s", logger->name);
> -		if (ret < 0)
> -			return ret;
> -		if (i == 0 && loggers[*pos][i + 1] != NULL) {
> -			ret = seq_printf(s, ",");
> -			if (ret < 0)
> -				return ret;
> -		}
> +		seq_printf(s, "%s", logger->name);
> +		if (i == 0 && loggers[*pos][i + 1] != NULL)
> +			seq_printf(s, ",");
> +
> +		if (seq_has_overflowed(s))
> +			return -ENOSPC;
>  	}
>  
> -	return seq_printf(s, ")\n");
> +	seq_printf(s, ")\n");
> +
> +	if (seq_has_overflowed(s))
> +		return -ENOSPC;
> +	return 0;
>  }
>  
>  static const struct seq_operations nflog_seq_ops = {
> diff --git a/net/netfilter/nfnetlink_queue_core.c b/net/netfilter/nfnetlink_queue_core.c
> index a82077d9f59b..f823f1538c4f 100644
> --- a/net/netfilter/nfnetlink_queue_core.c
> +++ b/net/netfilter/nfnetlink_queue_core.c
> @@ -1242,12 +1242,13 @@ static int seq_show(struct seq_file *s, void *v)
>  {
>  	const struct nfqnl_instance *inst = v;
>  
> -	return seq_printf(s, "%5d %6d %5d %1d %5d %5d %5d %8d %2d\n",
> -			  inst->queue_num,
> -			  inst->peer_portid, inst->queue_total,
> -			  inst->copy_mode, inst->copy_range,
> -			  inst->queue_dropped, inst->queue_user_dropped,
> -			  inst->id_sequence, 1);
> +	seq_printf(s, "%5d %6d %5d %1d %5d %5d %5d %8d %2d\n",
> +		   inst->queue_num,
> +		   inst->peer_portid, inst->queue_total,
> +		   inst->copy_mode, inst->copy_range,
> +		   inst->queue_dropped, inst->queue_user_dropped,
> +		   inst->id_sequence, 1);
> +	return seq_has_overflowed(s);
>  }
>  
>  static const struct seq_operations nfqnl_seq_ops = {
> diff --git a/net/netfilter/x_tables.c b/net/netfilter/x_tables.c
> index 133eb4772f12..51a459c3c649 100644
> --- a/net/netfilter/x_tables.c
> +++ b/net/netfilter/x_tables.c
> @@ -947,9 +947,10 @@ static int xt_table_seq_show(struct seq_file *seq, void *v)
>  {
>  	struct xt_table *table = list_entry(v, struct xt_table, list);
>  
> -	if (strlen(table->name))
> -		return seq_printf(seq, "%s\n", table->name);
> -	else
> +	if (strlen(table->name)) {
> +		seq_printf(seq, "%s\n", table->name);
> +		return seq_has_overflowed(seq);
> +	} else
>  		return 0;
>  }
>  
> @@ -1086,8 +1087,10 @@ static int xt_match_seq_show(struct seq_file *seq, void *v)
>  		if (trav->curr == trav->head)
>  			return 0;
>  		match = list_entry(trav->curr, struct xt_match, list);
> -		return (*match->name == '\0') ? 0 :
> -		       seq_printf(seq, "%s\n", match->name);
> +		if (*match->name == '\0')
> +			return 0;
> +		seq_printf(seq, "%s\n", match->name);
> +		return seq_has_overflowed(seq);
>  	}
>  	return 0;
>  }
> @@ -1139,8 +1142,10 @@ static int xt_target_seq_show(struct seq_file *seq, void *v)
>  		if (trav->curr == trav->head)
>  			return 0;
>  		target = list_entry(trav->curr, struct xt_target, list);
> -		return (*target->name == '\0') ? 0 :
> -		       seq_printf(seq, "%s\n", target->name);
> +		if (*target->name == '\0')
> +			return 0;
> +		seq_printf(seq, "%s\n", target->name);
> +		return seq_has_overflowed(seq);
>  	}
>  	return 0;
>  }
> diff --git a/net/netfilter/xt_hashlimit.c b/net/netfilter/xt_hashlimit.c
> index 05fbc2a0be46..178696852bde 100644
> --- a/net/netfilter/xt_hashlimit.c
> +++ b/net/netfilter/xt_hashlimit.c
> @@ -789,7 +789,6 @@ static void dl_seq_stop(struct seq_file *s, void *v)
>  static int dl_seq_real_show(struct dsthash_ent *ent, u_int8_t family,
>  				   struct seq_file *s)
>  {
> -	int res;
>  	const struct xt_hashlimit_htable *ht = s->private;
>  
>  	spin_lock(&ent->lock);
> @@ -798,33 +797,32 @@ static int dl_seq_real_show(struct dsthash_ent *ent, u_int8_t family,
>  
>  	switch (family) {
>  	case NFPROTO_IPV4:
> -		res = seq_printf(s, "%ld %pI4:%u->%pI4:%u %u %u %u\n",
> -				 (long)(ent->expires - jiffies)/HZ,
> -				 &ent->dst.ip.src,
> -				 ntohs(ent->dst.src_port),
> -				 &ent->dst.ip.dst,
> -				 ntohs(ent->dst.dst_port),
> -				 ent->rateinfo.credit, ent->rateinfo.credit_cap,
> -				 ent->rateinfo.cost);
> +		seq_printf(s, "%ld %pI4:%u->%pI4:%u %u %u %u\n",
> +			   (long)(ent->expires - jiffies)/HZ,
> +			   &ent->dst.ip.src,
> +			   ntohs(ent->dst.src_port),
> +			   &ent->dst.ip.dst,
> +			   ntohs(ent->dst.dst_port),
> +			   ent->rateinfo.credit, ent->rateinfo.credit_cap,
> +			   ent->rateinfo.cost);
>  		break;
>  #if IS_ENABLED(CONFIG_IP6_NF_IPTABLES)
>  	case NFPROTO_IPV6:
> -		res = seq_printf(s, "%ld %pI6:%u->%pI6:%u %u %u %u\n",
> -				 (long)(ent->expires - jiffies)/HZ,
> -				 &ent->dst.ip6.src,
> -				 ntohs(ent->dst.src_port),
> -				 &ent->dst.ip6.dst,
> -				 ntohs(ent->dst.dst_port),
> -				 ent->rateinfo.credit, ent->rateinfo.credit_cap,
> -				 ent->rateinfo.cost);
> +		seq_printf(s, "%ld %pI6:%u->%pI6:%u %u %u %u\n",
> +			   (long)(ent->expires - jiffies)/HZ,
> +			   &ent->dst.ip6.src,
> +			   ntohs(ent->dst.src_port),
> +			   &ent->dst.ip6.dst,
> +			   ntohs(ent->dst.dst_port),
> +			   ent->rateinfo.credit, ent->rateinfo.credit_cap,
> +			   ent->rateinfo.cost);
>  		break;
>  #endif
>  	default:
>  		BUG();
> -		res = 0;
>  	}
>  	spin_unlock(&ent->lock);
> -	return res;
> +	return seq_has_overflowed(s);
>  }
>  
>  static int dl_seq_show(struct seq_file *s, void *v)

[RFA][PATCH 5/8] dlm: Remove seq_printf() return checks and use seq_has_overflowed()

[Steven Rostedt]

[-- Attachment #1: 0005-dlm-Remove-seq_printf-return-checks-and-use-seq_has_.patch --]
[-- Type: text/plain, Size: 13292 bytes --]

From: Joe Perches <joe@perches.com>

[ REQUEST FOR ACKS ]

The seq_printf() return is going away soon and users of it should
check seq_has_overflowed() to see if the buffer is full and will
not accept any more data.

Convert functions returning int to void where seq_printf() is used.

Link: http://lkml.kernel.org/p/43590057bcb83846acbbcc1fe641f792b2fb7773.1412031505.git.joe@perches.com

Cc: Christine Caulfield <ccaulfie@redhat.com>
Cc: David Teigland <teigland@redhat.com>
Cc: cluster-devel@redhat.com
Signed-off-by: Joe Perches <joe@perches.com>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 fs/dlm/debug_fs.c | 251 +++++++++++++++++++++++++-----------------------------
 1 file changed, 117 insertions(+), 134 deletions(-)

diff --git a/fs/dlm/debug_fs.c b/fs/dlm/debug_fs.c
index 1323c568e362..3bf460894088 100644
--- a/fs/dlm/debug_fs.c
+++ b/fs/dlm/debug_fs.c
@@ -48,8 +48,8 @@ static char *print_lockmode(int mode)
 	}
 }
 
-static int print_format1_lock(struct seq_file *s, struct dlm_lkb *lkb,
-			      struct dlm_rsb *res)
+static void print_format1_lock(struct seq_file *s, struct dlm_lkb *lkb,
+			       struct dlm_rsb *res)
 {
 	seq_printf(s, "%08x %s", lkb->lkb_id, print_lockmode(lkb->lkb_grmode));
 
@@ -68,21 +68,17 @@ static int print_format1_lock(struct seq_file *s, struct dlm_lkb *lkb,
 	if (lkb->lkb_wait_type)
 		seq_printf(s, " wait_type: %d", lkb->lkb_wait_type);
 
-	return seq_puts(s, "\n");
+	seq_puts(s, "\n");
 }
 
-static int print_format1(struct dlm_rsb *res, struct seq_file *s)
+static void print_format1(struct dlm_rsb *res, struct seq_file *s)
 {
 	struct dlm_lkb *lkb;
 	int i, lvblen = res->res_ls->ls_lvblen, recover_list, root_list;
-	int rv;
 
 	lock_rsb(res);
 
-	rv = seq_printf(s, "\nResource %p Name (len=%d) \"",
-			res, res->res_length);
-	if (rv)
-		goto out;
+	seq_printf(s, "\nResource %p Name (len=%d) \"", res, res->res_length);
 
 	for (i = 0; i < res->res_length; i++) {
 		if (isprint(res->res_name[i]))
@@ -92,17 +88,16 @@ static int print_format1(struct dlm_rsb *res, struct seq_file *s)
 	}
 
 	if (res->res_nodeid > 0)
-		rv = seq_printf(s, "\"\nLocal Copy, Master is node %d\n",
-				res->res_nodeid);
+		seq_printf(s, "\"\nLocal Copy, Master is node %d\n",
+			   res->res_nodeid);
 	else if (res->res_nodeid == 0)
-		rv = seq_puts(s, "\"\nMaster Copy\n");
+		seq_puts(s, "\"\nMaster Copy\n");
 	else if (res->res_nodeid == -1)
-		rv = seq_printf(s, "\"\nLooking up master (lkid %x)\n",
-			   	res->res_first_lkid);
+		seq_printf(s, "\"\nLooking up master (lkid %x)\n",
+			   res->res_first_lkid);
 	else
-		rv = seq_printf(s, "\"\nInvalid master %d\n",
-				res->res_nodeid);
-	if (rv)
+		seq_printf(s, "\"\nInvalid master %d\n", res->res_nodeid);
+	if (seq_has_overflowed(s))
 		goto out;
 
 	/* Print the LVB: */
@@ -116,8 +111,8 @@ static int print_format1(struct dlm_rsb *res, struct seq_file *s)
 		}
 		if (rsb_flag(res, RSB_VALNOTVALID))
 			seq_puts(s, " (INVALID)");
-		rv = seq_puts(s, "\n");
-		if (rv)
+		seq_puts(s, "\n");
+		if (seq_has_overflowed(s))
 			goto out;
 	}
 
@@ -125,32 +120,30 @@ static int print_format1(struct dlm_rsb *res, struct seq_file *s)
 	recover_list = !list_empty(&res->res_recover_list);
 
 	if (root_list || recover_list) {
-		rv = seq_printf(s, "Recovery: root %d recover %d flags %lx "
-				"count %d\n", root_list, recover_list,
-			   	res->res_flags, res->res_recover_locks_count);
-		if (rv)
-			goto out;
+		seq_printf(s, "Recovery: root %d recover %d flags %lx count %d\n",
+			   root_list, recover_list,
+			   res->res_flags, res->res_recover_locks_count);
 	}
 
 	/* Print the locks attached to this resource */
 	seq_puts(s, "Granted Queue\n");
 	list_for_each_entry(lkb, &res->res_grantqueue, lkb_statequeue) {
-		rv = print_format1_lock(s, lkb, res);
-		if (rv)
+		print_format1_lock(s, lkb, res);
+		if (seq_has_overflowed(s))
 			goto out;
 	}
 
 	seq_puts(s, "Conversion Queue\n");
 	list_for_each_entry(lkb, &res->res_convertqueue, lkb_statequeue) {
-		rv = print_format1_lock(s, lkb, res);
-		if (rv)
+		print_format1_lock(s, lkb, res);
+		if (seq_has_overflowed(s))
 			goto out;
 	}
 
 	seq_puts(s, "Waiting Queue\n");
 	list_for_each_entry(lkb, &res->res_waitqueue, lkb_statequeue) {
-		rv = print_format1_lock(s, lkb, res);
-		if (rv)
+		print_format1_lock(s, lkb, res);
+		if (seq_has_overflowed(s))
 			goto out;
 	}
 
@@ -159,23 +152,23 @@ static int print_format1(struct dlm_rsb *res, struct seq_file *s)
 
 	seq_puts(s, "Lookup Queue\n");
 	list_for_each_entry(lkb, &res->res_lookup, lkb_rsb_lookup) {
-		rv = seq_printf(s, "%08x %s", lkb->lkb_id,
-				print_lockmode(lkb->lkb_rqmode));
+		seq_printf(s, "%08x %s",
+			   lkb->lkb_id, print_lockmode(lkb->lkb_rqmode));
 		if (lkb->lkb_wait_type)
 			seq_printf(s, " wait_type: %d", lkb->lkb_wait_type);
-		rv = seq_puts(s, "\n");
+		seq_puts(s, "\n");
+		if (seq_has_overflowed(s))
+			goto out;
 	}
  out:
 	unlock_rsb(res);
-	return rv;
 }
 
-static int print_format2_lock(struct seq_file *s, struct dlm_lkb *lkb,
-			      struct dlm_rsb *r)
+static void print_format2_lock(struct seq_file *s, struct dlm_lkb *lkb,
+			       struct dlm_rsb *r)
 {
 	u64 xid = 0;
 	u64 us;
-	int rv;
 
 	if (lkb->lkb_flags & DLM_IFL_USER) {
 		if (lkb->lkb_ua)
@@ -188,103 +181,97 @@ static int print_format2_lock(struct seq_file *s, struct dlm_lkb *lkb,
 	/* id nodeid remid pid xid exflags flags sts grmode rqmode time_us
 	   r_nodeid r_len r_name */
 
-	rv = seq_printf(s, "%x %d %x %u %llu %x %x %d %d %d %llu %u %d \"%s\"\n",
-			lkb->lkb_id,
-			lkb->lkb_nodeid,
-			lkb->lkb_remid,
-			lkb->lkb_ownpid,
-			(unsigned long long)xid,
-			lkb->lkb_exflags,
-			lkb->lkb_flags,
-			lkb->lkb_status,
-			lkb->lkb_grmode,
-			lkb->lkb_rqmode,
-			(unsigned long long)us,
-			r->res_nodeid,
-			r->res_length,
-			r->res_name);
-	return rv;
+	seq_printf(s, "%x %d %x %u %llu %x %x %d %d %d %llu %u %d \"%s\"\n",
+		   lkb->lkb_id,
+		   lkb->lkb_nodeid,
+		   lkb->lkb_remid,
+		   lkb->lkb_ownpid,
+		   (unsigned long long)xid,
+		   lkb->lkb_exflags,
+		   lkb->lkb_flags,
+		   lkb->lkb_status,
+		   lkb->lkb_grmode,
+		   lkb->lkb_rqmode,
+		   (unsigned long long)us,
+		   r->res_nodeid,
+		   r->res_length,
+		   r->res_name);
 }
 
-static int print_format2(struct dlm_rsb *r, struct seq_file *s)
+static void print_format2(struct dlm_rsb *r, struct seq_file *s)
 {
 	struct dlm_lkb *lkb;
-	int rv = 0;
 
 	lock_rsb(r);
 
 	list_for_each_entry(lkb, &r->res_grantqueue, lkb_statequeue) {
-		rv = print_format2_lock(s, lkb, r);
-		if (rv)
+		print_format2_lock(s, lkb, r);
+		if (seq_has_overflowed(s))
 			goto out;
 	}
 
 	list_for_each_entry(lkb, &r->res_convertqueue, lkb_statequeue) {
-		rv = print_format2_lock(s, lkb, r);
-		if (rv)
+		print_format2_lock(s, lkb, r);
+		if (seq_has_overflowed(s))
 			goto out;
 	}
 
 	list_for_each_entry(lkb, &r->res_waitqueue, lkb_statequeue) {
-		rv = print_format2_lock(s, lkb, r);
-		if (rv)
+		print_format2_lock(s, lkb, r);
+		if (seq_has_overflowed(s))
 			goto out;
 	}
  out:
 	unlock_rsb(r);
-	return rv;
 }
 
-static int print_format3_lock(struct seq_file *s, struct dlm_lkb *lkb,
+static void print_format3_lock(struct seq_file *s, struct dlm_lkb *lkb,
 			      int rsb_lookup)
 {
 	u64 xid = 0;
-	int rv;
 
 	if (lkb->lkb_flags & DLM_IFL_USER) {
 		if (lkb->lkb_ua)
 			xid = lkb->lkb_ua->xid;
 	}
 
-	rv = seq_printf(s, "lkb %x %d %x %u %llu %x %x %d %d %d %d %d %d %u %llu %llu\n",
-			lkb->lkb_id,
-			lkb->lkb_nodeid,
-			lkb->lkb_remid,
-			lkb->lkb_ownpid,
-			(unsigned long long)xid,
-			lkb->lkb_exflags,
-			lkb->lkb_flags,
-			lkb->lkb_status,
-			lkb->lkb_grmode,
-			lkb->lkb_rqmode,
-			lkb->lkb_last_bast.mode,
-			rsb_lookup,
-			lkb->lkb_wait_type,
-			lkb->lkb_lvbseq,
-			(unsigned long long)ktime_to_ns(lkb->lkb_timestamp),
-			(unsigned long long)ktime_to_ns(lkb->lkb_last_bast_time));
-	return rv;
+	seq_printf(s, "lkb %x %d %x %u %llu %x %x %d %d %d %d %d %d %u %llu %llu\n",
+		   lkb->lkb_id,
+		   lkb->lkb_nodeid,
+		   lkb->lkb_remid,
+		   lkb->lkb_ownpid,
+		   (unsigned long long)xid,
+		   lkb->lkb_exflags,
+		   lkb->lkb_flags,
+		   lkb->lkb_status,
+		   lkb->lkb_grmode,
+		   lkb->lkb_rqmode,
+		   lkb->lkb_last_bast.mode,
+		   rsb_lookup,
+		   lkb->lkb_wait_type,
+		   lkb->lkb_lvbseq,
+		   (unsigned long long)ktime_to_ns(lkb->lkb_timestamp),
+		   (unsigned long long)ktime_to_ns(lkb->lkb_last_bast_time));
 }
 
-static int print_format3(struct dlm_rsb *r, struct seq_file *s)
+static void print_format3(struct dlm_rsb *r, struct seq_file *s)
 {
 	struct dlm_lkb *lkb;
 	int i, lvblen = r->res_ls->ls_lvblen;
 	int print_name = 1;
-	int rv;
 
 	lock_rsb(r);
 
-	rv = seq_printf(s, "rsb %p %d %x %lx %d %d %u %d ",
-			r,
-			r->res_nodeid,
-			r->res_first_lkid,
-			r->res_flags,
-			!list_empty(&r->res_root_list),
-			!list_empty(&r->res_recover_list),
-			r->res_recover_locks_count,
-			r->res_length);
-	if (rv)
+	seq_printf(s, "rsb %p %d %x %lx %d %d %u %d ",
+		   r,
+		   r->res_nodeid,
+		   r->res_first_lkid,
+		   r->res_flags,
+		   !list_empty(&r->res_root_list),
+		   !list_empty(&r->res_recover_list),
+		   r->res_recover_locks_count,
+		   r->res_length);
+	if (seq_has_overflowed(s))
 		goto out;
 
 	for (i = 0; i < r->res_length; i++) {
@@ -300,8 +287,8 @@ static int print_format3(struct dlm_rsb *r, struct seq_file *s)
 		else
 			seq_printf(s, " %02x", (unsigned char)r->res_name[i]);
 	}
-	rv = seq_puts(s, "\n");
-	if (rv)
+	seq_puts(s, "\n");
+	if (seq_has_overflowed(s))
 		goto out;
 
 	if (!r->res_lvbptr)
@@ -311,58 +298,55 @@ static int print_format3(struct dlm_rsb *r, struct seq_file *s)
 
 	for (i = 0; i < lvblen; i++)
 		seq_printf(s, " %02x", (unsigned char)r->res_lvbptr[i]);
-	rv = seq_puts(s, "\n");
-	if (rv)
+	seq_puts(s, "\n");
+	if (seq_has_overflowed(s))
 		goto out;
 
  do_locks:
 	list_for_each_entry(lkb, &r->res_grantqueue, lkb_statequeue) {
-		rv = print_format3_lock(s, lkb, 0);
-		if (rv)
+		print_format3_lock(s, lkb, 0);
+		if (seq_has_overflowed(s))
 			goto out;
 	}
 
 	list_for_each_entry(lkb, &r->res_convertqueue, lkb_statequeue) {
-		rv = print_format3_lock(s, lkb, 0);
-		if (rv)
+		print_format3_lock(s, lkb, 0);
+		if (seq_has_overflowed(s))
 			goto out;
 	}
 
 	list_for_each_entry(lkb, &r->res_waitqueue, lkb_statequeue) {
-		rv = print_format3_lock(s, lkb, 0);
-		if (rv)
+		print_format3_lock(s, lkb, 0);
+		if (seq_has_overflowed(s))
 			goto out;
 	}
 
 	list_for_each_entry(lkb, &r->res_lookup, lkb_rsb_lookup) {
-		rv = print_format3_lock(s, lkb, 1);
-		if (rv)
+		print_format3_lock(s, lkb, 1);
+		if (seq_has_overflowed(s))
 			goto out;
 	}
  out:
 	unlock_rsb(r);
-	return rv;
 }
 
-static int print_format4(struct dlm_rsb *r, struct seq_file *s)
+static void print_format4(struct dlm_rsb *r, struct seq_file *s)
 {
 	int our_nodeid = dlm_our_nodeid();
 	int print_name = 1;
-	int i, rv;
+	int i;
 
 	lock_rsb(r);
 
-	rv = seq_printf(s, "rsb %p %d %d %d %d %lu %lx %d ",
-			r,
-			r->res_nodeid,
-			r->res_master_nodeid,
-			r->res_dir_nodeid,
-			our_nodeid,
-			r->res_toss_time,
-			r->res_flags,
-			r->res_length);
-	if (rv)
-		goto out;
+	seq_printf(s, "rsb %p %d %d %d %d %lu %lx %d ",
+		   r,
+		   r->res_nodeid,
+		   r->res_master_nodeid,
+		   r->res_dir_nodeid,
+		   our_nodeid,
+		   r->res_toss_time,
+		   r->res_flags,
+		   r->res_length);
 
 	for (i = 0; i < r->res_length; i++) {
 		if (!isascii(r->res_name[i]) || !isprint(r->res_name[i]))
@@ -377,10 +361,9 @@ static int print_format4(struct dlm_rsb *r, struct seq_file *s)
 		else
 			seq_printf(s, " %02x", (unsigned char)r->res_name[i]);
 	}
-	rv = seq_puts(s, "\n");
- out:
+	seq_puts(s, "\n");
+
 	unlock_rsb(r);
-	return rv;
 }
 
 struct rsbtbl_iter {
@@ -390,20 +373,20 @@ struct rsbtbl_iter {
 	int header;
 };
 
-/* seq_printf returns -1 if the buffer is full, and 0 otherwise.
-   If the buffer is full, seq_printf can be called again, but it
-   does nothing and just returns -1.  So, the these printing routines
-   periodically check the return value to avoid wasting too much time
-   trying to print to a full buffer. */
+/*
+ * If the buffer is full, seq_printf can be called again, but it
+ * does nothing.  So, the these printing routines periodically check
+ * seq_has_overflowed to avoid wasting too much time trying to print to
+ * a full buffer.
+ */
 
 static int table_seq_show(struct seq_file *seq, void *iter_ptr)
 {
 	struct rsbtbl_iter *ri = iter_ptr;
-	int rv = 0;
 
 	switch (ri->format) {
 	case 1:
-		rv = print_format1(ri->rsb, seq);
+		print_format1(ri->rsb, seq);
 		break;
 	case 2:
 		if (ri->header) {
@@ -412,25 +395,25 @@ static int table_seq_show(struct seq_file *seq, void *iter_ptr)
 					"r_nodeid r_len r_name\n");
 			ri->header = 0;
 		}
-		rv = print_format2(ri->rsb, seq);
+		print_format2(ri->rsb, seq);
 		break;
 	case 3:
 		if (ri->header) {
 			seq_printf(seq, "version rsb 1.1 lvb 1.1 lkb 1.1\n");
 			ri->header = 0;
 		}
-		rv = print_format3(ri->rsb, seq);
+		print_format3(ri->rsb, seq);
 		break;
 	case 4:
 		if (ri->header) {
 			seq_printf(seq, "version 4 rsb 2\n");
 			ri->header = 0;
 		}
-		rv = print_format4(ri->rsb, seq);
+		print_format4(ri->rsb, seq);
 		break;
 	}
 
-	return rv;
+	return 0;
 }
 
 static const struct seq_operations format1_seq_ops;
-- 
2.1.1

Re: [RFA][PATCH 5/8] dlm: Remove seq_printf() return checks and use seq_has_overflowed()

[Steven Rostedt]

On Wed, 29 Oct 2014 17:56:07 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

> From: Joe Perches <joe@perches.com>
> 
> [ REQUEST FOR ACKS ]

Can any of the DLM maintainers give me an Acked-by for this?

Thanks!

-- Steve

> 
> The seq_printf() return is going away soon and users of it should
> check seq_has_overflowed() to see if the buffer is full and will
> not accept any more data.
> 
> Convert functions returning int to void where seq_printf() is used.
> 
> Link: http://lkml.kernel.org/p/43590057bcb83846acbbcc1fe641f792b2fb7773.1412031505.git.joe@perches.com
> 
> Cc: Christine Caulfield <ccaulfie@redhat.com>
> Cc: David Teigland <teigland@redhat.com>
> Cc: cluster-devel@redhat.com
> Signed-off-by: Joe Perches <joe@perches.com>
> Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
> ---
>  fs/dlm/debug_fs.c | 251 +++++++++++++++++++++++++-----------------------------
>  1 file changed, 117 insertions(+), 134 deletions(-)
> 
> diff --git a/fs/dlm/debug_fs.c b/fs/dlm/debug_fs.c
> index 1323c568e362..3bf460894088 100644
> --- a/fs/dlm/debug_fs.c
> +++ b/fs/dlm/debug_fs.c
> @@ -48,8 +48,8 @@ static char *print_lockmode(int mode)
>  	}
>  }
>  
> -static int print_format1_lock(struct seq_file *s, struct dlm_lkb *lkb,
> -			      struct dlm_rsb *res)
> +static void print_format1_lock(struct seq_file *s, struct dlm_lkb *lkb,
> +			       struct dlm_rsb *res)
>  {
>  	seq_printf(s, "%08x %s", lkb->lkb_id, print_lockmode(lkb->lkb_grmode));
>  
> @@ -68,21 +68,17 @@ static int print_format1_lock(struct seq_file *s, struct dlm_lkb *lkb,
>  	if (lkb->lkb_wait_type)
>  		seq_printf(s, " wait_type: %d", lkb->lkb_wait_type);
>  
> -	return seq_puts(s, "\n");
> +	seq_puts(s, "\n");
>  }
>  
> -static int print_format1(struct dlm_rsb *res, struct seq_file *s)
> +static void print_format1(struct dlm_rsb *res, struct seq_file *s)
>  {
>  	struct dlm_lkb *lkb;
>  	int i, lvblen = res->res_ls->ls_lvblen, recover_list, root_list;
> -	int rv;
>  
>  	lock_rsb(res);
>  
> -	rv = seq_printf(s, "\nResource %p Name (len=%d) \"",
> -			res, res->res_length);
> -	if (rv)
> -		goto out;
> +	seq_printf(s, "\nResource %p Name (len=%d) \"", res, res->res_length);
>  
>  	for (i = 0; i < res->res_length; i++) {
>  		if (isprint(res->res_name[i]))
> @@ -92,17 +88,16 @@ static int print_format1(struct dlm_rsb *res, struct seq_file *s)
>  	}
>  
>  	if (res->res_nodeid > 0)
> -		rv = seq_printf(s, "\"\nLocal Copy, Master is node %d\n",
> -				res->res_nodeid);
> +		seq_printf(s, "\"\nLocal Copy, Master is node %d\n",
> +			   res->res_nodeid);
>  	else if (res->res_nodeid == 0)
> -		rv = seq_puts(s, "\"\nMaster Copy\n");
> +		seq_puts(s, "\"\nMaster Copy\n");
>  	else if (res->res_nodeid == -1)
> -		rv = seq_printf(s, "\"\nLooking up master (lkid %x)\n",
> -			   	res->res_first_lkid);
> +		seq_printf(s, "\"\nLooking up master (lkid %x)\n",
> +			   res->res_first_lkid);
>  	else
> -		rv = seq_printf(s, "\"\nInvalid master %d\n",
> -				res->res_nodeid);
> -	if (rv)
> +		seq_printf(s, "\"\nInvalid master %d\n", res->res_nodeid);
> +	if (seq_has_overflowed(s))
>  		goto out;
>  
>  	/* Print the LVB: */
> @@ -116,8 +111,8 @@ static int print_format1(struct dlm_rsb *res, struct seq_file *s)
>  		}
>  		if (rsb_flag(res, RSB_VALNOTVALID))
>  			seq_puts(s, " (INVALID)");
> -		rv = seq_puts(s, "\n");
> -		if (rv)
> +		seq_puts(s, "\n");
> +		if (seq_has_overflowed(s))
>  			goto out;
>  	}
>  
> @@ -125,32 +120,30 @@ static int print_format1(struct dlm_rsb *res, struct seq_file *s)
>  	recover_list = !list_empty(&res->res_recover_list);
>  
>  	if (root_list || recover_list) {
> -		rv = seq_printf(s, "Recovery: root %d recover %d flags %lx "
> -				"count %d\n", root_list, recover_list,
> -			   	res->res_flags, res->res_recover_locks_count);
> -		if (rv)
> -			goto out;
> +		seq_printf(s, "Recovery: root %d recover %d flags %lx count %d\n",
> +			   root_list, recover_list,
> +			   res->res_flags, res->res_recover_locks_count);
>  	}
>  
>  	/* Print the locks attached to this resource */
>  	seq_puts(s, "Granted Queue\n");
>  	list_for_each_entry(lkb, &res->res_grantqueue, lkb_statequeue) {
> -		rv = print_format1_lock(s, lkb, res);
> -		if (rv)
> +		print_format1_lock(s, lkb, res);
> +		if (seq_has_overflowed(s))
>  			goto out;
>  	}
>  
>  	seq_puts(s, "Conversion Queue\n");
>  	list_for_each_entry(lkb, &res->res_convertqueue, lkb_statequeue) {
> -		rv = print_format1_lock(s, lkb, res);
> -		if (rv)
> +		print_format1_lock(s, lkb, res);
> +		if (seq_has_overflowed(s))
>  			goto out;
>  	}
>  
>  	seq_puts(s, "Waiting Queue\n");
>  	list_for_each_entry(lkb, &res->res_waitqueue, lkb_statequeue) {
> -		rv = print_format1_lock(s, lkb, res);
> -		if (rv)
> +		print_format1_lock(s, lkb, res);
> +		if (seq_has_overflowed(s))
>  			goto out;
>  	}
>  
> @@ -159,23 +152,23 @@ static int print_format1(struct dlm_rsb *res, struct seq_file *s)
>  
>  	seq_puts(s, "Lookup Queue\n");
>  	list_for_each_entry(lkb, &res->res_lookup, lkb_rsb_lookup) {
> -		rv = seq_printf(s, "%08x %s", lkb->lkb_id,
> -				print_lockmode(lkb->lkb_rqmode));
> +		seq_printf(s, "%08x %s",
> +			   lkb->lkb_id, print_lockmode(lkb->lkb_rqmode));
>  		if (lkb->lkb_wait_type)
>  			seq_printf(s, " wait_type: %d", lkb->lkb_wait_type);
> -		rv = seq_puts(s, "\n");
> +		seq_puts(s, "\n");
> +		if (seq_has_overflowed(s))
> +			goto out;
>  	}
>   out:
>  	unlock_rsb(res);
> -	return rv;
>  }
>  
> -static int print_format2_lock(struct seq_file *s, struct dlm_lkb *lkb,
> -			      struct dlm_rsb *r)
> +static void print_format2_lock(struct seq_file *s, struct dlm_lkb *lkb,
> +			       struct dlm_rsb *r)
>  {
>  	u64 xid = 0;
>  	u64 us;
> -	int rv;
>  
>  	if (lkb->lkb_flags & DLM_IFL_USER) {
>  		if (lkb->lkb_ua)
> @@ -188,103 +181,97 @@ static int print_format2_lock(struct seq_file *s, struct dlm_lkb *lkb,
>  	/* id nodeid remid pid xid exflags flags sts grmode rqmode time_us
>  	   r_nodeid r_len r_name */
>  
> -	rv = seq_printf(s, "%x %d %x %u %llu %x %x %d %d %d %llu %u %d \"%s\"\n",
> -			lkb->lkb_id,
> -			lkb->lkb_nodeid,
> -			lkb->lkb_remid,
> -			lkb->lkb_ownpid,
> -			(unsigned long long)xid,
> -			lkb->lkb_exflags,
> -			lkb->lkb_flags,
> -			lkb->lkb_status,
> -			lkb->lkb_grmode,
> -			lkb->lkb_rqmode,
> -			(unsigned long long)us,
> -			r->res_nodeid,
> -			r->res_length,
> -			r->res_name);
> -	return rv;
> +	seq_printf(s, "%x %d %x %u %llu %x %x %d %d %d %llu %u %d \"%s\"\n",
> +		   lkb->lkb_id,
> +		   lkb->lkb_nodeid,
> +		   lkb->lkb_remid,
> +		   lkb->lkb_ownpid,
> +		   (unsigned long long)xid,
> +		   lkb->lkb_exflags,
> +		   lkb->lkb_flags,
> +		   lkb->lkb_status,
> +		   lkb->lkb_grmode,
> +		   lkb->lkb_rqmode,
> +		   (unsigned long long)us,
> +		   r->res_nodeid,
> +		   r->res_length,
> +		   r->res_name);
>  }
>  
> -static int print_format2(struct dlm_rsb *r, struct seq_file *s)
> +static void print_format2(struct dlm_rsb *r, struct seq_file *s)
>  {
>  	struct dlm_lkb *lkb;
> -	int rv = 0;
>  
>  	lock_rsb(r);
>  
>  	list_for_each_entry(lkb, &r->res_grantqueue, lkb_statequeue) {
> -		rv = print_format2_lock(s, lkb, r);
> -		if (rv)
> +		print_format2_lock(s, lkb, r);
> +		if (seq_has_overflowed(s))
>  			goto out;
>  	}
>  
>  	list_for_each_entry(lkb, &r->res_convertqueue, lkb_statequeue) {
> -		rv = print_format2_lock(s, lkb, r);
> -		if (rv)
> +		print_format2_lock(s, lkb, r);
> +		if (seq_has_overflowed(s))
>  			goto out;
>  	}
>  
>  	list_for_each_entry(lkb, &r->res_waitqueue, lkb_statequeue) {
> -		rv = print_format2_lock(s, lkb, r);
> -		if (rv)
> +		print_format2_lock(s, lkb, r);
> +		if (seq_has_overflowed(s))
>  			goto out;
>  	}
>   out:
>  	unlock_rsb(r);
> -	return rv;
>  }
>  
> -static int print_format3_lock(struct seq_file *s, struct dlm_lkb *lkb,
> +static void print_format3_lock(struct seq_file *s, struct dlm_lkb *lkb,
>  			      int rsb_lookup)
>  {
>  	u64 xid = 0;
> -	int rv;
>  
>  	if (lkb->lkb_flags & DLM_IFL_USER) {
>  		if (lkb->lkb_ua)
>  			xid = lkb->lkb_ua->xid;
>  	}
>  
> -	rv = seq_printf(s, "lkb %x %d %x %u %llu %x %x %d %d %d %d %d %d %u %llu %llu\n",
> -			lkb->lkb_id,
> -			lkb->lkb_nodeid,
> -			lkb->lkb_remid,
> -			lkb->lkb_ownpid,
> -			(unsigned long long)xid,
> -			lkb->lkb_exflags,
> -			lkb->lkb_flags,
> -			lkb->lkb_status,
> -			lkb->lkb_grmode,
> -			lkb->lkb_rqmode,
> -			lkb->lkb_last_bast.mode,
> -			rsb_lookup,
> -			lkb->lkb_wait_type,
> -			lkb->lkb_lvbseq,
> -			(unsigned long long)ktime_to_ns(lkb->lkb_timestamp),
> -			(unsigned long long)ktime_to_ns(lkb->lkb_last_bast_time));
> -	return rv;
> +	seq_printf(s, "lkb %x %d %x %u %llu %x %x %d %d %d %d %d %d %u %llu %llu\n",
> +		   lkb->lkb_id,
> +		   lkb->lkb_nodeid,
> +		   lkb->lkb_remid,
> +		   lkb->lkb_ownpid,
> +		   (unsigned long long)xid,
> +		   lkb->lkb_exflags,
> +		   lkb->lkb_flags,
> +		   lkb->lkb_status,
> +		   lkb->lkb_grmode,
> +		   lkb->lkb_rqmode,
> +		   lkb->lkb_last_bast.mode,
> +		   rsb_lookup,
> +		   lkb->lkb_wait_type,
> +		   lkb->lkb_lvbseq,
> +		   (unsigned long long)ktime_to_ns(lkb->lkb_timestamp),
> +		   (unsigned long long)ktime_to_ns(lkb->lkb_last_bast_time));
>  }
>  
> -static int print_format3(struct dlm_rsb *r, struct seq_file *s)
> +static void print_format3(struct dlm_rsb *r, struct seq_file *s)
>  {
>  	struct dlm_lkb *lkb;
>  	int i, lvblen = r->res_ls->ls_lvblen;
>  	int print_name = 1;
> -	int rv;
>  
>  	lock_rsb(r);
>  
> -	rv = seq_printf(s, "rsb %p %d %x %lx %d %d %u %d ",
> -			r,
> -			r->res_nodeid,
> -			r->res_first_lkid,
> -			r->res_flags,
> -			!list_empty(&r->res_root_list),
> -			!list_empty(&r->res_recover_list),
> -			r->res_recover_locks_count,
> -			r->res_length);
> -	if (rv)
> +	seq_printf(s, "rsb %p %d %x %lx %d %d %u %d ",
> +		   r,
> +		   r->res_nodeid,
> +		   r->res_first_lkid,
> +		   r->res_flags,
> +		   !list_empty(&r->res_root_list),
> +		   !list_empty(&r->res_recover_list),
> +		   r->res_recover_locks_count,
> +		   r->res_length);
> +	if (seq_has_overflowed(s))
>  		goto out;
>  
>  	for (i = 0; i < r->res_length; i++) {
> @@ -300,8 +287,8 @@ static int print_format3(struct dlm_rsb *r, struct seq_file *s)
>  		else
>  			seq_printf(s, " %02x", (unsigned char)r->res_name[i]);
>  	}
> -	rv = seq_puts(s, "\n");
> -	if (rv)
> +	seq_puts(s, "\n");
> +	if (seq_has_overflowed(s))
>  		goto out;
>  
>  	if (!r->res_lvbptr)
> @@ -311,58 +298,55 @@ static int print_format3(struct dlm_rsb *r, struct seq_file *s)
>  
>  	for (i = 0; i < lvblen; i++)
>  		seq_printf(s, " %02x", (unsigned char)r->res_lvbptr[i]);
> -	rv = seq_puts(s, "\n");
> -	if (rv)
> +	seq_puts(s, "\n");
> +	if (seq_has_overflowed(s))
>  		goto out;
>  
>   do_locks:
>  	list_for_each_entry(lkb, &r->res_grantqueue, lkb_statequeue) {
> -		rv = print_format3_lock(s, lkb, 0);
> -		if (rv)
> +		print_format3_lock(s, lkb, 0);
> +		if (seq_has_overflowed(s))
>  			goto out;
>  	}
>  
>  	list_for_each_entry(lkb, &r->res_convertqueue, lkb_statequeue) {
> -		rv = print_format3_lock(s, lkb, 0);
> -		if (rv)
> +		print_format3_lock(s, lkb, 0);
> +		if (seq_has_overflowed(s))
>  			goto out;
>  	}
>  
>  	list_for_each_entry(lkb, &r->res_waitqueue, lkb_statequeue) {
> -		rv = print_format3_lock(s, lkb, 0);
> -		if (rv)
> +		print_format3_lock(s, lkb, 0);
> +		if (seq_has_overflowed(s))
>  			goto out;
>  	}
>  
>  	list_for_each_entry(lkb, &r->res_lookup, lkb_rsb_lookup) {
> -		rv = print_format3_lock(s, lkb, 1);
> -		if (rv)
> +		print_format3_lock(s, lkb, 1);
> +		if (seq_has_overflowed(s))
>  			goto out;
>  	}
>   out:
>  	unlock_rsb(r);
> -	return rv;
>  }
>  
> -static int print_format4(struct dlm_rsb *r, struct seq_file *s)
> +static void print_format4(struct dlm_rsb *r, struct seq_file *s)
>  {
>  	int our_nodeid = dlm_our_nodeid();
>  	int print_name = 1;
> -	int i, rv;
> +	int i;
>  
>  	lock_rsb(r);
>  
> -	rv = seq_printf(s, "rsb %p %d %d %d %d %lu %lx %d ",
> -			r,
> -			r->res_nodeid,
> -			r->res_master_nodeid,
> -			r->res_dir_nodeid,
> -			our_nodeid,
> -			r->res_toss_time,
> -			r->res_flags,
> -			r->res_length);
> -	if (rv)
> -		goto out;
> +	seq_printf(s, "rsb %p %d %d %d %d %lu %lx %d ",
> +		   r,
> +		   r->res_nodeid,
> +		   r->res_master_nodeid,
> +		   r->res_dir_nodeid,
> +		   our_nodeid,
> +		   r->res_toss_time,
> +		   r->res_flags,
> +		   r->res_length);
>  
>  	for (i = 0; i < r->res_length; i++) {
>  		if (!isascii(r->res_name[i]) || !isprint(r->res_name[i]))
> @@ -377,10 +361,9 @@ static int print_format4(struct dlm_rsb *r, struct seq_file *s)
>  		else
>  			seq_printf(s, " %02x", (unsigned char)r->res_name[i]);
>  	}
> -	rv = seq_puts(s, "\n");
> - out:
> +	seq_puts(s, "\n");
> +
>  	unlock_rsb(r);
> -	return rv;
>  }
>  
>  struct rsbtbl_iter {
> @@ -390,20 +373,20 @@ struct rsbtbl_iter {
>  	int header;
>  };
>  
> -/* seq_printf returns -1 if the buffer is full, and 0 otherwise.
> -   If the buffer is full, seq_printf can be called again, but it
> -   does nothing and just returns -1.  So, the these printing routines
> -   periodically check the return value to avoid wasting too much time
> -   trying to print to a full buffer. */
> +/*
> + * If the buffer is full, seq_printf can be called again, but it
> + * does nothing.  So, the these printing routines periodically check
> + * seq_has_overflowed to avoid wasting too much time trying to print to
> + * a full buffer.
> + */
>  
>  static int table_seq_show(struct seq_file *seq, void *iter_ptr)
>  {
>  	struct rsbtbl_iter *ri = iter_ptr;
> -	int rv = 0;
>  
>  	switch (ri->format) {
>  	case 1:
> -		rv = print_format1(ri->rsb, seq);
> +		print_format1(ri->rsb, seq);
>  		break;
>  	case 2:
>  		if (ri->header) {
> @@ -412,25 +395,25 @@ static int table_seq_show(struct seq_file *seq, void *iter_ptr)
>  					"r_nodeid r_len r_name\n");
>  			ri->header = 0;
>  		}
> -		rv = print_format2(ri->rsb, seq);
> +		print_format2(ri->rsb, seq);
>  		break;
>  	case 3:
>  		if (ri->header) {
>  			seq_printf(seq, "version rsb 1.1 lvb 1.1 lkb 1.1\n");
>  			ri->header = 0;
>  		}
> -		rv = print_format3(ri->rsb, seq);
> +		print_format3(ri->rsb, seq);
>  		break;
>  	case 4:
>  		if (ri->header) {
>  			seq_printf(seq, "version 4 rsb 2\n");
>  			ri->header = 0;
>  		}
> -		rv = print_format4(ri->rsb, seq);
> +		print_format4(ri->rsb, seq);
>  		break;
>  	}
>  
> -	return rv;
> +	return 0;
>  }
>  
>  static const struct seq_operations format1_seq_ops;

Re: [RFA][PATCH 5/8] dlm: Remove seq_printf() return checks and use seq_has_overflowed()

[David Teigland]

On Tue, Nov 04, 2014 at 08:08:52AM -0500, Steven Rostedt wrote:
> On Wed, 29 Oct 2014 17:56:07 -0400
> Steven Rostedt <rostedt@goodmis.org> wrote:
> 
> > From: Joe Perches <joe@perches.com>
> > 
> > [ REQUEST FOR ACKS ]
> 
> Can any of the DLM maintainers give me an Acked-by for this?

Looks ok,
Dave

[RFA][PATCH 6/8] dlm: Use seq_puts() instead of seq_printf() for constant strings

[Steven Rostedt]

[-- Attachment #1: 0006-dlm-Use-seq_puts-instead-of-seq_printf-for-constant-.patch --]
[-- Type: text/plain, Size: 2036 bytes --]

From: Joe Perches <joe@perches.com>

[ REQUEST FOR ACKS ]

Convert the seq_printf output with constant strings to seq_puts.

Link: http://lkml.kernel.org/p/b416b016f4a6e49115ba736cad6ea2709a8bc1c4.1412031505.git.joe@perches.com

Cc: Christine Caulfield <ccaulfie@redhat.com>
Cc: David Teigland <teigland@redhat.com>
Cc: cluster-devel@redhat.com
Signed-off-by: Joe Perches <joe@perches.com>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 fs/dlm/debug_fs.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/fs/dlm/debug_fs.c b/fs/dlm/debug_fs.c
index 3bf460894088..eea64912c9c0 100644
--- a/fs/dlm/debug_fs.c
+++ b/fs/dlm/debug_fs.c
@@ -279,7 +279,7 @@ static void print_format3(struct dlm_rsb *r, struct seq_file *s)
 			print_name = 0;
 	}
 
-	seq_printf(s, "%s", print_name ? "str " : "hex");
+	seq_puts(s, print_name ? "str " : "hex");
 
 	for (i = 0; i < r->res_length; i++) {
 		if (print_name)
@@ -353,7 +353,7 @@ static void print_format4(struct dlm_rsb *r, struct seq_file *s)
 			print_name = 0;
 	}
 
-	seq_printf(s, "%s", print_name ? "str " : "hex");
+	seq_puts(s, print_name ? "str " : "hex");
 
 	for (i = 0; i < r->res_length; i++) {
 		if (print_name)
@@ -390,23 +390,21 @@ static int table_seq_show(struct seq_file *seq, void *iter_ptr)
 		break;
 	case 2:
 		if (ri->header) {
-			seq_printf(seq, "id nodeid remid pid xid exflags "
-					"flags sts grmode rqmode time_ms "
-					"r_nodeid r_len r_name\n");
+			seq_puts(seq, "id nodeid remid pid xid exflags flags sts grmode rqmode time_ms r_nodeid r_len r_name\n");
 			ri->header = 0;
 		}
 		print_format2(ri->rsb, seq);
 		break;
 	case 3:
 		if (ri->header) {
-			seq_printf(seq, "version rsb 1.1 lvb 1.1 lkb 1.1\n");
+			seq_puts(seq, "version rsb 1.1 lvb 1.1 lkb 1.1\n");
 			ri->header = 0;
 		}
 		print_format3(ri->rsb, seq);
 		break;
 	case 4:
 		if (ri->header) {
-			seq_printf(seq, "version 4 rsb 2\n");
+			seq_puts(seq, "version 4 rsb 2\n");
 			ri->header = 0;
 		}
 		print_format4(ri->rsb, seq);
-- 
2.1.1

Re: [RFA][PATCH 6/8] dlm: Use seq_puts() instead of seq_printf() for constant strings

[Steven Rostedt]

On Wed, 29 Oct 2014 17:56:08 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

> From: Joe Perches <joe@perches.com>
> 
> [ REQUEST FOR ACKS ]

Can any of the DLM maintainers give me an Acked-by for this?

Thanks!

-- Steve

> 
> Convert the seq_printf output with constant strings to seq_puts.
> 
> Link: http://lkml.kernel.org/p/b416b016f4a6e49115ba736cad6ea2709a8bc1c4.1412031505.git.joe@perches.com
> 
> Cc: Christine Caulfield <ccaulfie@redhat.com>
> Cc: David Teigland <teigland@redhat.com>
> Cc: cluster-devel@redhat.com
> Signed-off-by: Joe Perches <joe@perches.com>
> Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
> ---
>  fs/dlm/debug_fs.c | 12 +++++-------
>  1 file changed, 5 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/dlm/debug_fs.c b/fs/dlm/debug_fs.c
> index 3bf460894088..eea64912c9c0 100644
> --- a/fs/dlm/debug_fs.c
> +++ b/fs/dlm/debug_fs.c
> @@ -279,7 +279,7 @@ static void print_format3(struct dlm_rsb *r, struct seq_file *s)
>  			print_name = 0;
>  	}
>  
> -	seq_printf(s, "%s", print_name ? "str " : "hex");
> +	seq_puts(s, print_name ? "str " : "hex");
>  
>  	for (i = 0; i < r->res_length; i++) {
>  		if (print_name)
> @@ -353,7 +353,7 @@ static void print_format4(struct dlm_rsb *r, struct seq_file *s)
>  			print_name = 0;
>  	}
>  
> -	seq_printf(s, "%s", print_name ? "str " : "hex");
> +	seq_puts(s, print_name ? "str " : "hex");
>  
>  	for (i = 0; i < r->res_length; i++) {
>  		if (print_name)
> @@ -390,23 +390,21 @@ static int table_seq_show(struct seq_file *seq, void *iter_ptr)
>  		break;
>  	case 2:
>  		if (ri->header) {
> -			seq_printf(seq, "id nodeid remid pid xid exflags "
> -					"flags sts grmode rqmode time_ms "
> -					"r_nodeid r_len r_name\n");
> +			seq_puts(seq, "id nodeid remid pid xid exflags flags sts grmode rqmode time_ms r_nodeid r_len r_name\n");
>  			ri->header = 0;
>  		}
>  		print_format2(ri->rsb, seq);
>  		break;
>  	case 3:
>  		if (ri->header) {
> -			seq_printf(seq, "version rsb 1.1 lvb 1.1 lkb 1.1\n");
> +			seq_puts(seq, "version rsb 1.1 lvb 1.1 lkb 1.1\n");
>  			ri->header = 0;
>  		}
>  		print_format3(ri->rsb, seq);
>  		break;
>  	case 4:
>  		if (ri->header) {
> -			seq_printf(seq, "version 4 rsb 2\n");
> +			seq_puts(seq, "version 4 rsb 2\n");
>  			ri->header = 0;
>  		}
>  		print_format4(ri->rsb, seq);

[RFA][PATCH 7/8] fs: Convert show_fdinfo functions to void

[Steven Rostedt]

[-- Attachment #1: 0007-fs-Convert-show_fdinfo-functions-to-void.patch --]
[-- Type: text/plain, Size: 13177 bytes --]

From: Joe Perches <joe@perches.com>

[ REQUEST FOR ACKS ]

seq_printf functions shouldn't really check the return value.
Checking seq_has_overflowed() occasionally is used instead.

Update vfs documentation.

Link: http://lkml.kernel.org/p/e37e6e7b76acbdcc3bb4ab2a57c8f8ca1ae11b9a.1412031505.git.joe@perches.com

Cc: David S. Miller <davem@davemloft.net>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Joe Perches <joe@perches.com>
[ did a few clean ups ]
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 Documentation/filesystems/vfs.txt |  2 +-
 drivers/net/tun.c                 |  4 +-
 fs/eventfd.c                      |  9 ++---
 fs/eventpoll.c                    | 13 +++----
 fs/notify/fdinfo.c                | 78 ++++++++++++++++-----------------------
 fs/notify/fdinfo.h                |  4 +-
 fs/proc/fd.c                      |  3 +-
 fs/signalfd.c                     |  4 +-
 fs/timerfd.c                      | 27 +++++++-------
 include/linux/fs.h                |  2 +-
 10 files changed, 63 insertions(+), 83 deletions(-)

diff --git a/Documentation/filesystems/vfs.txt b/Documentation/filesystems/vfs.txt
index fceff7c00a3c..af1dbc1823bb 100644
--- a/Documentation/filesystems/vfs.txt
+++ b/Documentation/filesystems/vfs.txt
@@ -828,7 +828,7 @@ struct file_operations {
 	ssize_t (*splice_read)(struct file *, struct pipe_inode_info *, size_t, unsigned int);
 	int (*setlease)(struct file *, long arg, struct file_lock **, void **);
 	long (*fallocate)(struct file *, int mode, loff_t offset, loff_t len);
-	int (*show_fdinfo)(struct seq_file *m, struct file *f);
+	void (*show_fdinfo)(struct seq_file *m, struct file *f);
 };
 
 Again, all methods are called without any locks being held, unless
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index 186ce541c657..a3420e091689 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -2209,7 +2209,7 @@ static int tun_chr_close(struct inode *inode, struct file *file)
 }
 
 #ifdef CONFIG_PROC_FS
-static int tun_chr_show_fdinfo(struct seq_file *m, struct file *f)
+static void tun_chr_show_fdinfo(struct seq_file *m, struct file *f)
 {
 	struct tun_struct *tun;
 	struct ifreq ifr;
@@ -2225,7 +2225,7 @@ static int tun_chr_show_fdinfo(struct seq_file *m, struct file *f)
 	if (tun)
 		tun_put(tun);
 
-	return seq_printf(m, "iff:\t%s\n", ifr.ifr_name);
+	seq_printf(m, "iff:\t%s\n", ifr.ifr_name);
 }
 #endif
 
diff --git a/fs/eventfd.c b/fs/eventfd.c
index d6a88e7812f3..4b0a226024fa 100644
--- a/fs/eventfd.c
+++ b/fs/eventfd.c
@@ -287,17 +287,14 @@ static ssize_t eventfd_write(struct file *file, const char __user *buf, size_t c
 }
 
 #ifdef CONFIG_PROC_FS
-static int eventfd_show_fdinfo(struct seq_file *m, struct file *f)
+static void eventfd_show_fdinfo(struct seq_file *m, struct file *f)
 {
 	struct eventfd_ctx *ctx = f->private_data;
-	int ret;
 
 	spin_lock_irq(&ctx->wqh.lock);
-	ret = seq_printf(m, "eventfd-count: %16llx\n",
-			 (unsigned long long)ctx->count);
+	seq_printf(m, "eventfd-count: %16llx\n",
+		   (unsigned long long)ctx->count);
 	spin_unlock_irq(&ctx->wqh.lock);
-
-	return ret;
 }
 #endif
 
diff --git a/fs/eventpoll.c b/fs/eventpoll.c
index 7bcfff900f05..d77f94491352 100644
--- a/fs/eventpoll.c
+++ b/fs/eventpoll.c
@@ -870,25 +870,22 @@ static unsigned int ep_eventpoll_poll(struct file *file, poll_table *wait)
 }
 
 #ifdef CONFIG_PROC_FS
-static int ep_show_fdinfo(struct seq_file *m, struct file *f)
+static void ep_show_fdinfo(struct seq_file *m, struct file *f)
 {
 	struct eventpoll *ep = f->private_data;
 	struct rb_node *rbp;
-	int ret = 0;
 
 	mutex_lock(&ep->mtx);
 	for (rbp = rb_first(&ep->rbr); rbp; rbp = rb_next(rbp)) {
 		struct epitem *epi = rb_entry(rbp, struct epitem, rbn);
 
-		ret = seq_printf(m, "tfd: %8d events: %8x data: %16llx\n",
-				 epi->ffd.fd, epi->event.events,
-				 (long long)epi->event.data);
-		if (ret)
+		seq_printf(m, "tfd: %8d events: %8x data: %16llx\n",
+			   epi->ffd.fd, epi->event.events,
+			   (long long)epi->event.data);
+		if (seq_has_overflowed(m))
 			break;
 	}
 	mutex_unlock(&ep->mtx);
-
-	return ret;
 }
 #endif
 
diff --git a/fs/notify/fdinfo.c b/fs/notify/fdinfo.c
index 9d7e2b9659cb..6ffd220eb14d 100644
--- a/fs/notify/fdinfo.c
+++ b/fs/notify/fdinfo.c
@@ -20,25 +20,24 @@
 
 #if defined(CONFIG_INOTIFY_USER) || defined(CONFIG_FANOTIFY)
 
-static int show_fdinfo(struct seq_file *m, struct file *f,
-		       int (*show)(struct seq_file *m, struct fsnotify_mark *mark))
+static void show_fdinfo(struct seq_file *m, struct file *f,
+			void (*show)(struct seq_file *m,
+				     struct fsnotify_mark *mark))
 {
 	struct fsnotify_group *group = f->private_data;
 	struct fsnotify_mark *mark;
-	int ret = 0;
 
 	mutex_lock(&group->mark_mutex);
 	list_for_each_entry(mark, &group->marks_list, g_list) {
-		ret = show(m, mark);
-		if (ret)
+		show(m, mark);
+		if (seq_has_overflowed(m))
 			break;
 	}
 	mutex_unlock(&group->mark_mutex);
-	return ret;
 }
 
 #if defined(CONFIG_EXPORTFS)
-static int show_mark_fhandle(struct seq_file *m, struct inode *inode)
+static void show_mark_fhandle(struct seq_file *m, struct inode *inode)
 {
 	struct {
 		struct file_handle handle;
@@ -52,71 +51,62 @@ static int show_mark_fhandle(struct seq_file *m, struct inode *inode)
 	ret = exportfs_encode_inode_fh(inode, (struct fid *)f.handle.f_handle, &size, 0);
 	if ((ret == FILEID_INVALID) || (ret < 0)) {
 		WARN_ONCE(1, "Can't encode file handler for inotify: %d\n", ret);
-		return 0;
+		return;
 	}
 
 	f.handle.handle_type = ret;
 	f.handle.handle_bytes = size * sizeof(u32);
 
-	ret = seq_printf(m, "fhandle-bytes:%x fhandle-type:%x f_handle:",
-			 f.handle.handle_bytes, f.handle.handle_type);
+	seq_printf(m, "fhandle-bytes:%x fhandle-type:%x f_handle:",
+		   f.handle.handle_bytes, f.handle.handle_type);
 
 	for (i = 0; i < f.handle.handle_bytes; i++)
-		ret |= seq_printf(m, "%02x", (int)f.handle.f_handle[i]);
-
-	return ret;
+		seq_printf(m, "%02x", (int)f.handle.f_handle[i]);
 }
 #else
-static int show_mark_fhandle(struct seq_file *m, struct inode *inode)
+static void show_mark_fhandle(struct seq_file *m, struct inode *inode)
 {
-	return 0;
 }
 #endif
 
 #ifdef CONFIG_INOTIFY_USER
 
-static int inotify_fdinfo(struct seq_file *m, struct fsnotify_mark *mark)
+static void inotify_fdinfo(struct seq_file *m, struct fsnotify_mark *mark)
 {
 	struct inotify_inode_mark *inode_mark;
 	struct inode *inode;
-	int ret = 0;
 
 	if (!(mark->flags & (FSNOTIFY_MARK_FLAG_ALIVE | FSNOTIFY_MARK_FLAG_INODE)))
-		return 0;
+		return;
 
 	inode_mark = container_of(mark, struct inotify_inode_mark, fsn_mark);
 	inode = igrab(mark->i.inode);
 	if (inode) {
-		ret = seq_printf(m, "inotify wd:%x ino:%lx sdev:%x "
-				 "mask:%x ignored_mask:%x ",
-				 inode_mark->wd, inode->i_ino,
-				 inode->i_sb->s_dev,
-				 mark->mask, mark->ignored_mask);
-		ret |= show_mark_fhandle(m, inode);
-		ret |= seq_putc(m, '\n');
+		seq_printf(m, "inotify wd:%x ino:%lx sdev:%x mask:%x ignored_mask:%x ",
+			   inode_mark->wd, inode->i_ino, inode->i_sb->s_dev,
+			   mark->mask, mark->ignored_mask);
+		show_mark_fhandle(m, inode);
+		seq_putc(m, '\n');
 		iput(inode);
 	}
-
-	return ret;
 }
 
-int inotify_show_fdinfo(struct seq_file *m, struct file *f)
+void inotify_show_fdinfo(struct seq_file *m, struct file *f)
 {
-	return show_fdinfo(m, f, inotify_fdinfo);
+	show_fdinfo(m, f, inotify_fdinfo);
 }
 
 #endif /* CONFIG_INOTIFY_USER */
 
 #ifdef CONFIG_FANOTIFY
 
-static int fanotify_fdinfo(struct seq_file *m, struct fsnotify_mark *mark)
+static void fanotify_fdinfo(struct seq_file *m, struct fsnotify_mark *mark)
 {
 	unsigned int mflags = 0;
 	struct inode *inode;
-	int ret = 0;
 
 	if (!(mark->flags & FSNOTIFY_MARK_FLAG_ALIVE))
-		return 0;
+		return;
 
 	if (mark->flags & FSNOTIFY_MARK_FLAG_IGNORED_SURV_MODIFY)
 		mflags |= FAN_MARK_IGNORED_SURV_MODIFY;
@@ -124,26 +114,22 @@ static int fanotify_fdinfo(struct seq_file *m, struct fsnotify_mark *mark)
 	if (mark->flags & FSNOTIFY_MARK_FLAG_INODE) {
 		inode = igrab(mark->i.inode);
 		if (!inode)
-			goto out;
-		ret = seq_printf(m, "fanotify ino:%lx sdev:%x "
-				 "mflags:%x mask:%x ignored_mask:%x ",
-				 inode->i_ino, inode->i_sb->s_dev,
-				 mflags, mark->mask, mark->ignored_mask);
-		ret |= show_mark_fhandle(m, inode);
-		ret |= seq_putc(m, '\n');
+			return;
+		seq_printf(m, "fanotify ino:%lx sdev:%x mflags:%x mask:%x ignored_mask:%x ",
+			   inode->i_ino, inode->i_sb->s_dev,
+			   mflags, mark->mask, mark->ignored_mask);
+		show_mark_fhandle(m, inode);
+		seq_putc(m, '\n');
 		iput(inode);
 	} else if (mark->flags & FSNOTIFY_MARK_FLAG_VFSMOUNT) {
 		struct mount *mnt = real_mount(mark->m.mnt);
 
-		ret = seq_printf(m, "fanotify mnt_id:%x mflags:%x mask:%x "
-				 "ignored_mask:%x\n", mnt->mnt_id, mflags,
-				 mark->mask, mark->ignored_mask);
+		seq_printf(m, "fanotify mnt_id:%x mflags:%x mask:%x ignored_mask:%x\n",
+			   mnt->mnt_id, mflags, mark->mask, mark->ignored_mask);
 	}
-out:
-	return ret;
 }
 
-int fanotify_show_fdinfo(struct seq_file *m, struct file *f)
+void fanotify_show_fdinfo(struct seq_file *m, struct file *f)
 {
 	struct fsnotify_group *group = f->private_data;
 	unsigned int flags = 0;
@@ -169,7 +155,7 @@ int fanotify_show_fdinfo(struct seq_file *m, struct file *f)
 	seq_printf(m, "fanotify flags:%x event-flags:%x\n",
 		   flags, group->fanotify_data.f_flags);
 
-	return show_fdinfo(m, f, fanotify_fdinfo);
+	show_fdinfo(m, f, fanotify_fdinfo);
 }
 
 #endif /* CONFIG_FANOTIFY */
diff --git a/fs/notify/fdinfo.h b/fs/notify/fdinfo.h
index 556afda990e9..9664c4904d6b 100644
--- a/fs/notify/fdinfo.h
+++ b/fs/notify/fdinfo.h
@@ -10,11 +10,11 @@ struct file;
 #ifdef CONFIG_PROC_FS
 
 #ifdef CONFIG_INOTIFY_USER
-extern int inotify_show_fdinfo(struct seq_file *m, struct file *f);
+void inotify_show_fdinfo(struct seq_file *m, struct file *f);
 #endif
 
 #ifdef CONFIG_FANOTIFY
-extern int fanotify_show_fdinfo(struct seq_file *m, struct file *f);
+void fanotify_show_fdinfo(struct seq_file *m, struct file *f);
 #endif
 
 #else /* CONFIG_PROC_FS */
diff --git a/fs/proc/fd.c b/fs/proc/fd.c
index e11d7c590bb0..8e5ad83b629a 100644
--- a/fs/proc/fd.c
+++ b/fs/proc/fd.c
@@ -53,7 +53,8 @@ static int seq_show(struct seq_file *m, void *v)
 			   (long long)file->f_pos, f_flags,
 			   real_mount(file->f_path.mnt)->mnt_id);
 		if (file->f_op->show_fdinfo)
-			ret = file->f_op->show_fdinfo(m, file);
+			file->f_op->show_fdinfo(m, file);
+		ret = seq_has_overflowed(m);
 		fput(file);
 	}
 
diff --git a/fs/signalfd.c b/fs/signalfd.c
index 424b7b65321f..7e412ad74836 100644
--- a/fs/signalfd.c
+++ b/fs/signalfd.c
@@ -230,7 +230,7 @@ static ssize_t signalfd_read(struct file *file, char __user *buf, size_t count,
 }
 
 #ifdef CONFIG_PROC_FS
-static int signalfd_show_fdinfo(struct seq_file *m, struct file *f)
+static void signalfd_show_fdinfo(struct seq_file *m, struct file *f)
 {
 	struct signalfd_ctx *ctx = f->private_data;
 	sigset_t sigmask;
@@ -238,8 +238,6 @@ static int signalfd_show_fdinfo(struct seq_file *m, struct file *f)
 	sigmask = ctx->sigmask;
 	signotset(&sigmask);
 	render_sigset_t(m, "sigmask:\t", &sigmask);
-
-	return 0;
 }
 #endif
 
diff --git a/fs/timerfd.c b/fs/timerfd.c
index b46ffa94372a..b94fa6c3c6eb 100644
--- a/fs/timerfd.c
+++ b/fs/timerfd.c
@@ -288,7 +288,7 @@ static ssize_t timerfd_read(struct file *file, char __user *buf, size_t count,
 }
 
 #ifdef CONFIG_PROC_FS
-static int timerfd_show(struct seq_file *m, struct file *file)
+static void timerfd_show(struct seq_file *m, struct file *file)
 {
 	struct timerfd_ctx *ctx = file->private_data;
 	struct itimerspec t;
@@ -298,18 +298,19 @@ static int timerfd_show(struct seq_file *m, struct file *file)
 	t.it_interval = ktime_to_timespec(ctx->tintv);
 	spin_unlock_irq(&ctx->wqh.lock);
 
-	return seq_printf(m,
-			  "clockid: %d\n"
-			  "ticks: %llu\n"
-			  "settime flags: 0%o\n"
-			  "it_value: (%llu, %llu)\n"
-			  "it_interval: (%llu, %llu)\n",
-			  ctx->clockid, (unsigned long long)ctx->ticks,
-			  ctx->settime_flags,
-			  (unsigned long long)t.it_value.tv_sec,
-			  (unsigned long long)t.it_value.tv_nsec,
-			  (unsigned long long)t.it_interval.tv_sec,
-			  (unsigned long long)t.it_interval.tv_nsec);
+	seq_printf(m,
+		   "clockid: %d\n"
+		   "ticks: %llu\n"
+		   "settime flags: 0%o\n"
+		   "it_value: (%llu, %llu)\n"
+		   "it_interval: (%llu, %llu)\n",
+		   ctx->clockid,
+		   (unsigned long long)ctx->ticks,
+		   ctx->settime_flags,
+		   (unsigned long long)t.it_value.tv_sec,
+		   (unsigned long long)t.it_value.tv_nsec,
+		   (unsigned long long)t.it_interval.tv_sec,
+		   (unsigned long long)t.it_interval.tv_nsec);
 }
 #else
 #define timerfd_show NULL
diff --git a/include/linux/fs.h b/include/linux/fs.h
index a957d4366c24..01dd9052a142 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1491,7 +1491,7 @@ struct file_operations {
 	int (*setlease)(struct file *, long, struct file_lock **, void **);
 	long (*fallocate)(struct file *file, int mode, loff_t offset,
 			  loff_t len);
-	int (*show_fdinfo)(struct seq_file *m, struct file *f);
+	void (*show_fdinfo)(struct seq_file *m, struct file *f);
 };
 
 struct inode_operations {
-- 
2.1.1

[RFA][PATCH 8/8] debugfs: Have debugfs_print_regs32() return void

[Steven Rostedt]

[-- Attachment #1: 0008-debugfs-Have-debugfs_print_regs32-return-void.patch --]
[-- Type: text/plain, Size: 3795 bytes --]

From: Joe Perches <joe@perches.com>

[ REQUEST FOR ACKS ]

The seq_printf() will soon just return void, and seq_has_overflowed()
should be used instead to see if the seq can no longer accept input.

As the return value of debugfs_print_regs32() has no users and
the seq_file descriptor should be checked with seq_has_overflowed()
instead of return values of functions, it is better to just have
debugfs_print_regs32() also return void.

Link: http://lkml.kernel.org/p/2634b19eb1c04a9d31148c1fe6f1f3819be95349.1412031505.git.joe@perches.com

Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Joe Perches <joe@perches.com>
[ original change only updated seq_printf() return, added return of
  void to debugfs_print_regs32() as well ]
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 Documentation/filesystems/debugfs.txt |  2 +-
 fs/debugfs/file.c                     | 15 ++++++++-------
 include/linux/debugfs.h               |  7 +++----
 3 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/Documentation/filesystems/debugfs.txt b/Documentation/filesystems/debugfs.txt
index 3a863f692728..88ab81c79109 100644
--- a/Documentation/filesystems/debugfs.txt
+++ b/Documentation/filesystems/debugfs.txt
@@ -140,7 +140,7 @@ file.
 				     struct dentry *parent,
 				     struct debugfs_regset32 *regset);
 
-    int debugfs_print_regs32(struct seq_file *s, struct debugfs_reg32 *regs,
+    void debugfs_print_regs32(struct seq_file *s, struct debugfs_reg32 *regs,
 			 int nregs, void __iomem *base, char *prefix);
 
 The "base" argument may be 0, but you may want to build the reg32 array
diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c
index 76c08c2beb2f..8e0f2f410189 100644
--- a/fs/debugfs/file.c
+++ b/fs/debugfs/file.c
@@ -692,18 +692,19 @@ EXPORT_SYMBOL_GPL(debugfs_create_u32_array);
  * because some peripherals have several blocks of identical registers,
  * for example configuration of dma channels
  */
-int debugfs_print_regs32(struct seq_file *s, const struct debugfs_reg32 *regs,
-			   int nregs, void __iomem *base, char *prefix)
+void debugfs_print_regs32(struct seq_file *s, const struct debugfs_reg32 *regs,
+			  int nregs, void __iomem *base, char *prefix)
 {
-	int i, ret = 0;
+	int i;
 
 	for (i = 0; i < nregs; i++, regs++) {
 		if (prefix)
-			ret += seq_printf(s, "%s", prefix);
-		ret += seq_printf(s, "%s = 0x%08x\n", regs->name,
-				  readl(base + regs->offset));
+			seq_printf(s, "%s", prefix);
+		seq_printf(s, "%s = 0x%08x\n", regs->name,
+			   readl(base + regs->offset));
+		if (seq_has_overflowed(s))
+			break;
 	}
-	return ret;
 }
 EXPORT_SYMBOL_GPL(debugfs_print_regs32);
 
diff --git a/include/linux/debugfs.h b/include/linux/debugfs.h
index 4d0b4d1aa132..d84f8c254a87 100644
--- a/include/linux/debugfs.h
+++ b/include/linux/debugfs.h
@@ -92,8 +92,8 @@ struct dentry *debugfs_create_regset32(const char *name, umode_t mode,
 				     struct dentry *parent,
 				     struct debugfs_regset32 *regset);
 
-int debugfs_print_regs32(struct seq_file *s, const struct debugfs_reg32 *regs,
-			 int nregs, void __iomem *base, char *prefix);
+void debugfs_print_regs32(struct seq_file *s, const struct debugfs_reg32 *regs,
+			  int nregs, void __iomem *base, char *prefix);
 
 struct dentry *debugfs_create_u32_array(const char *name, umode_t mode,
 					struct dentry *parent,
@@ -233,10 +233,9 @@ static inline struct dentry *debugfs_create_regset32(const char *name,
 	return ERR_PTR(-ENODEV);
 }
 
-static inline int debugfs_print_regs32(struct seq_file *s, const struct debugfs_reg32 *regs,
+static inline void debugfs_print_regs32(struct seq_file *s, const struct debugfs_reg32 *regs,
 			 int nregs, void __iomem *base, char *prefix)
 {
-	return 0;
 }
 
 static inline bool debugfs_initialized(void)
-- 
2.1.1

Re: [RFA][PATCH 8/8] debugfs: Have debugfs_print_regs32() return void

[Greg Kroah-Hartman]

On Wed, Oct 29, 2014 at 05:56:10PM -0400, Steven Rostedt wrote:
> From: Joe Perches <joe@perches.com>
> 
> [ REQUEST FOR ACKS ]
> 
> The seq_printf() will soon just return void, and seq_has_overflowed()
> should be used instead to see if the seq can no longer accept input.
> 
> As the return value of debugfs_print_regs32() has no users and
> the seq_file descriptor should be checked with seq_has_overflowed()
> instead of return values of functions, it is better to just have
> debugfs_print_regs32() also return void.
> 
> Link: http://lkml.kernel.org/p/2634b19eb1c04a9d31148c1fe6f1f3819be95349.1412031505.git.joe@perches.com
> 
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Signed-off-by: Joe Perches <joe@perches.com>
> [ original change only updated seq_printf() return, added return of
>   void to debugfs_print_regs32() as well ]
> Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
> ---

Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Re: [RFA][PATCH 8/8] debugfs: Have debugfs_print_regs32() return void

[Steven Rostedt]

On Wed, 29 Oct 2014 15:03:15 -0700
Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:

> On Wed, Oct 29, 2014 at 05:56:10PM -0400, Steven Rostedt wrote:
> > From: Joe Perches <joe@perches.com>
> > 
> > [ REQUEST FOR ACKS ]
> > 
> > The seq_printf() will soon just return void, and seq_has_overflowed()
> > should be used instead to see if the seq can no longer accept input.
> > 
> > As the return value of debugfs_print_regs32() has no users and
> > the seq_file descriptor should be checked with seq_has_overflowed()
> > instead of return values of functions, it is better to just have
> > debugfs_print_regs32() also return void.
> > 
> > Link: http://lkml.kernel.org/p/2634b19eb1c04a9d31148c1fe6f1f3819be95349.1412031505.git.joe@perches.com
> > 
> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > Signed-off-by: Joe Perches <joe@perches.com>
> > [ original change only updated seq_printf() return, added return of
> >   void to debugfs_print_regs32() as well ]
> > Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
> > ---
> 
> Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Thanks!

-- Steve

Re: [RFA][PATCH 0/8] seq_file: Remove return checks of seq_printf()

[Steven Rostedt]

On Wed, 29 Oct 2014 17:56:02 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

> [ REQUEST FOR ACKS ]

Al,

Can you take a look at these, and give me an Acked-by if they are OK
with you?

Thanks,

-- Steve

> 
> I'm looking to clean up the seq_file code and to eventually merge the
> trace_seq code with seq_file as well, since they basically do the same thing.
> 
> Part of this process is to remove the return code of seq_printf() and friends
> as they are rather inconsistent. It is better to use the new function
> seq_has_overflowed() if you want to stop processing when the buffer
> is full. Note, if the buffer is full, the seq_file code will throw away
> the contents, allocate a bigger buffer, and then call your code again
> to fill in the data. The only thing that breaking out of the function
> early does is to save a little time which is probably never noticed.
> 
> But anyway, I'm asking for Acked-bys for code that touches the subsystems
> that have been modified.
> 
> I started with patches from Joe Perches and modified them as well.
> There's many more places that need to be updated before we can convert
> seq_printf() and friends to return void. But this patch set introduces
> the seq_has_overflowed() and does some initial updates.
> 
> Please ack!
> 
> Thanks!
> 
> -- Steve
> 
> 
> 
> Joe Perches (6):
>       seq_file: Rename seq_overflow() to seq_has_overflowed() and make public
>       netfilter: Convert print_tuple functions to return void
>       dlm: Remove seq_printf() return checks and use seq_has_overflowed()
>       dlm: Use seq_puts() instead of seq_printf() for constant strings
>       fs: Convert show_fdinfo functions to void
>       debugfs: Have debugfs_print_regs32() return void
> 
> Steven Rostedt (Red Hat) (2):
>       netfilter: Remove return values for print_conntrack callbacks
>       netfilter: Remove checks of seq_printf() return values
> 
> ----
>  Documentation/filesystems/debugfs.txt              |   2 +-
>  Documentation/filesystems/seq_file.txt             |  22 +-
>  Documentation/filesystems/vfs.txt                  |   2 +-
>  drivers/net/tun.c                                  |   4 +-
>  fs/debugfs/file.c                                  |  15 +-
>  fs/dlm/debug_fs.c                                  | 263 ++++++++++-----------
>  fs/eventfd.c                                       |   9 +-
>  fs/eventpoll.c                                     |  13 +-
>  fs/notify/fdinfo.c                                 |  78 +++---
>  fs/notify/fdinfo.h                                 |   4 +-
>  fs/proc/fd.c                                       |   3 +-
>  fs/seq_file.c                                      |  15 +-
>  fs/signalfd.c                                      |   4 +-
>  fs/timerfd.c                                       |  27 ++-
>  include/linux/debugfs.h                            |   7 +-
>  include/linux/fs.h                                 |   2 +-
>  include/linux/seq_file.h                           |  15 ++
>  include/net/netfilter/nf_conntrack_core.h          |   2 +-
>  include/net/netfilter/nf_conntrack_l3proto.h       |   4 +-
>  include/net/netfilter/nf_conntrack_l4proto.h       |   6 +-
>  net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c     |   6 +-
>  .../netfilter/nf_conntrack_l3proto_ipv4_compat.c   |  53 +++--
>  net/ipv4/netfilter/nf_conntrack_proto_icmp.c       |  10 +-
>  net/ipv6/netfilter/nf_conntrack_l3proto_ipv6.c     |   6 +-
>  net/ipv6/netfilter/nf_conntrack_proto_icmpv6.c     |  10 +-
>  net/netfilter/nf_conntrack_l3proto_generic.c       |   5 +-
>  net/netfilter/nf_conntrack_proto_dccp.c            |  14 +-
>  net/netfilter/nf_conntrack_proto_generic.c         |   5 +-
>  net/netfilter/nf_conntrack_proto_gre.c             |  18 +-
>  net/netfilter/nf_conntrack_proto_sctp.c            |  14 +-
>  net/netfilter/nf_conntrack_proto_tcp.c             |  14 +-
>  net/netfilter/nf_conntrack_proto_udp.c             |  10 +-
>  net/netfilter/nf_conntrack_proto_udplite.c         |  10 +-
>  net/netfilter/nf_conntrack_standalone.c            |  77 +++---
>  net/netfilter/nf_log.c                             |  30 +--
>  net/netfilter/nfnetlink_queue_core.c               |  13 +-
>  net/netfilter/x_tables.c                           |  19 +-
>  net/netfilter/xt_hashlimit.c                       |  36 ++-
>  38 files changed, 410 insertions(+), 437 deletions(-)

Re: [RFA][PATCH 0/8] seq_file: Remove return checks of seq_printf()

[Al Viro]

On Tue, Nov 04, 2014 at 08:04:36AM -0500, Steven Rostedt wrote:
> On Wed, 29 Oct 2014 17:56:02 -0400
> Steven Rostedt <rostedt@goodmis.org> wrote:
> 
> > [ REQUEST FOR ACKS ]
> 
> Al,
> 
> Can you take a look at these, and give me an Acked-by if they are OK
> with you?

Looks sane.  How do you want to handle the merge of that series?

Re: [RFA][PATCH 0/8] seq_file: Remove return checks of seq_printf()

[Steven Rostedt]

On Wed, 5 Nov 2014 17:50:21 +0000
Al Viro <viro@ZenIV.linux.org.uk> wrote:

> On Tue, Nov 04, 2014 at 08:04:36AM -0500, Steven Rostedt wrote:
> > On Wed, 29 Oct 2014 17:56:02 -0400
> > Steven Rostedt <rostedt@goodmis.org> wrote:
> > 
> > > [ REQUEST FOR ACKS ]
> > 
> > Al,
> > 
> > Can you take a look at these, and give me an Acked-by if they are OK
> > with you?
> 
> Looks sane.  How do you want to handle the merge of that series?

I can push them to Linus with your Acked-by, or if you want, you can
take them as well. You can either use the patches from the list or I can
make a git branch for you to pull from.

-- Steve