Skip to contentSkip to conversation

seq_file: Remove return checks of seq_printf()

PATCH In review Steven Rostedt 7 participants 40 messages Oct 29, 2014 – Nov 5, 2014

Subsystems (11)
lore
Share & tools
lore
#0 lore raw

cover seq_file: Remove return checks of seq_printf()

[ 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(-)
#1 lore raw

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

From: Joe Perches <[email protected]>

[ 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().

Cc Al Viro <[email protected]>
[ Reworked the original patch from Joe ]
---
 Documentation/filesystems/seq_file.txt | 22 +++++++++++++---------
 fs/seq_file.c                          | 15 ++-------------
 include/linux/seq_file.h               | 15 +++++++++++++++
 3 files changed, 30 insertions(+), 22 deletions(-)
Documentation/filesystems/seq_file.txt +13 -9
index 8ea3e90ace07..79b4037a86e6 100644
@@ -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
 
fs/seq_file.c +2 -13
index 3857b720cb1b..353948ba1c5b 100644
@@ -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;
include/linux/seq_file.h +15 -0
index 52e0097f61f0..07c98e1998c3 100644
@@ -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
Signature · 1 line
2.1.1
#2 lore raw

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

Review

Quoted 2 lines · Steven Rostedt
(quoted diff) +2 -0
+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?

#3 lore raw

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

Quoted 10 lines · On Wed, 29 Oct 2014 15:08:36 -0700 Joe Perches <[email protected]> wrote:

Review

Quoted 2 lines · Steven Rostedt
(quoted diff) +2 -0
+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

#4 lore raw

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

Quoted 8 lines · Steven Rostedt
Quoted 7 lines · On Wed, 29 Oct 2014 15:08:36 -0700 Joe Perches <[email protected]> wrote:

Review

Quoted 2 lines · Steven Rostedt
(quoted diff) +2 -0
+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?

Quoted 7 lines

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.

Quoted 3 lines

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?

#5 lore raw

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

Quoted 13 lines · On Wed, 29 Oct 2014 16:53:30 -0700 Joe Perches <[email protected]> wrote:
Quoted 12 lines · Steven Rostedt
Quoted 7 lines · On Wed, 29 Oct 2014 15:08:36 -0700 Joe Perches <[email protected]> wrote:

Review

Quoted 2 lines · Steven Rostedt
(quoted diff) +2 -0
+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.

Quoted 7 lines
Quoted 3 lines

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.

Quoted 7 lines
Quoted 4 lines

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

#6 lore raw

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

Quoted 20 lines · Steven Rostedt
Quoted 17 lines · On Wed, 29 Oct 2014 16:53:30 -0700 Joe Perches <[email protected]> wrote:
Quoted 15 lines · Steven Rostedt
Quoted 7 lines · On Wed, 29 Oct 2014 15:08:36 -0700 Joe Perches <[email protected]> wrote:

Review

Quoted 2 lines · Steven Rostedt
(quoted diff) +2 -0
+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.

Quoted 8 lines
Quoted 3 lines

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.

#7 lore raw

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

In reply to Joe Perches
Quoted 10 lines · On Wed, 29 Oct 2014 15:08:36 -0700 Joe Perches <[email protected]> wrote:

Review

Quoted 2 lines · Steven Rostedt
(quoted diff) +2 -0
+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

#8 lore raw

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

Quoted 1 line · On Wed, 29 Oct 2014 20:20:45 -0400 Steven Rostedt <[email protected]> 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().

Cc Al Viro <[email protected]>
[ Reworked the original patch from Joe ]
---
 Documentation/filesystems/seq_file.txt | 22 +++++++++++++---------
 fs/seq_file.c                          | 15 ++-------------
 include/linux/seq_file.h               | 15 +++++++++++++++
 3 files changed, 30 insertions(+), 22 deletions(-)
Documentation/filesystems/seq_file.txt +13 -9
index 8ea3e90ace07..b797ed38de46 100644
@@ -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
 
fs/seq_file.c +2 -13
index 3857b720cb1b..353948ba1c5b 100644
@@ -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;
include/linux/seq_file.h +15 -0
index 52e0097f61f0..cf6a9daaaf6d 100644
@@ -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
Signature · 1 line
1.8.1.4
#9 lore raw

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

Quoted 5 lines · Steven Rostedt
Quoted 1 line · On Wed, 29 Oct 2014 20:20:45 -0400 Steven Rostedt <[email protected]> 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

Quoted 3 lines

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

#10 lore raw

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

In reply to Steven Rostedt

From: "Steven Rostedt (Red Hat)" <[email protected]>

[ 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 <[email protected]>
Cc Patrick McHardy <[email protected]>
Cc Jozsef Kadlecsik <[email protected]>
---
 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(-)
include/net/netfilter/nf_conntrack_l4proto.h +1 -1
index 4c8d573830b7..82e4ec002a39 100644
@@ -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);
net/ipv4/netfilter/nf_conntrack_l3proto_ipv4_compat.c +4 -1
index 4c48e434bb1f..91f207c2cb69 100644
@@ -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,
net/netfilter/nf_conntrack_proto_dccp.c +2 -2
index cb372f96f10d..15971177470a 100644
@@ -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)
net/netfilter/nf_conntrack_proto_gre.c +4 -4
index d5665739e3b1..cba607ada069 100644
@@ -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)
net/netfilter/nf_conntrack_proto_sctp.c +2 -2
index 1314d33f6bcf..c61f4cd6407d 100644
@@ -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)	\
net/netfilter/nf_conntrack_proto_tcp.c +2 -2
index 44d1ea32570a..79668fd3db96 100644
@@ -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)
net/netfilter/nf_conntrack_standalone.c +2 -2
index cf65a1e040dd..348aa3602787 100644
@@ -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))
Signature · 1 line
2.1.1
maintainer
#11 lore raw

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

Quoted 8 lines · Steven Rostedt

From: "Steven Rostedt (Red Hat)" <[email protected]>

[ 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.

[..]

Quoted 5 lines
net/ipv4/netfilter/nf_conntrack_l3proto_ipv4_compat.c +0 -0
index 4c48e434bb1f..91f207c2cb69 100644

net/ipv4/netfilter/nf_conntrack_l3proto_ipv4_compat.c static int ct_seq_show(struct seq_file *s, void *v) · line 154

Quoted 9 lines
net/ipv4/netfilter/nf_conntrack_l3proto_ipv4_compat.c +4 -1
@@ -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 ...

Quoted 5 lines

net/netfilter/nf_conntrack_standalone.c

Quoted 1 line
net/netfilter/nf_conntrack_standalone.c +0 -0

index cf65a1e040dd..348aa3602787 100644

net/netfilter/nf_conntrack_standalone.c static int ct_seq_show(struct seq_file *s, void *v) · line 206

Quoted 10 lines
net/netfilter/nf_conntrack_standalone.c +2 -2
@@ -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.

#12 lore raw

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

Quoted 42 lines · On Wed, 29 Oct 2014 23:16:01 +0100 Florian Westphal <[email protected]> wrote:
Quoted 8 lines · Steven Rostedt

From: "Steven Rostedt (Red Hat)" <[email protected]>

[ 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.

[..]

Quoted 5 lines
net/ipv4/netfilter/nf_conntrack_l3proto_ipv4_compat.c +0 -0
index 4c48e434bb1f..91f207c2cb69 100644

net/ipv4/netfilter/nf_conntrack_l3proto_ipv4_compat.c static int ct_seq_show(struct seq_file *s, void *v) · line 154

Quoted 9 lines
net/ipv4/netfilter/nf_conntrack_l3proto_ipv4_compat.c +4 -1
@@ -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 ...

Quoted 13 lines

net/netfilter/nf_conntrack_standalone.c

Quoted 1 line
net/netfilter/nf_conntrack_standalone.c +0 -0

index cf65a1e040dd..348aa3602787 100644

net/netfilter/nf_conntrack_standalone.c +2 -2
@@ -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);
 
Quoted 4 lines

net/netfilter/nf_conntrack_standalone.c static int ct_seq_show(struct seq_file *s, void *v) · line 206

Quoted 2 lines
net/netfilter/nf_conntrack_standalone.c +0 -0
 	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

#13 lore raw

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

In reply to Florian Westphal
Quoted 42 lines · On Wed, 29 Oct 2014 23:16:01 +0100 Florian Westphal <[email protected]> wrote:
Quoted 8 lines · Steven Rostedt

From: "Steven Rostedt (Red Hat)" <[email protected]>

[ 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.

[..]

Quoted 5 lines
net/ipv4/netfilter/nf_conntrack_l3proto_ipv4_compat.c +0 -0
index 4c48e434bb1f..91f207c2cb69 100644

net/ipv4/netfilter/nf_conntrack_l3proto_ipv4_compat.c static int ct_seq_show(struct seq_file *s, void *v) · line 154

Quoted 9 lines
net/ipv4/netfilter/nf_conntrack_l3proto_ipv4_compat.c +4 -1
@@ -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 ...

Quoted 13 lines

net/netfilter/nf_conntrack_standalone.c

Quoted 1 line
net/netfilter/nf_conntrack_standalone.c +0 -0

index cf65a1e040dd..348aa3602787 100644

net/netfilter/nf_conntrack_standalone.c +2 -2
@@ -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);
 
Quoted 4 lines

net/netfilter/nf_conntrack_standalone.c static int ct_seq_show(struct seq_file *s, void *v) · line 206

Quoted 2 lines
net/netfilter/nf_conntrack_standalone.c +0 -0
 	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

#14 lore raw

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

In reply to Steven Rostedt
Quoted 3 lines · On Wed, 29 Oct 2014 17:56:04 -0400 Steven Rostedt <[email protected]> wrote:

From: "Steven Rostedt (Red Hat)" <[email protected]>

[ REQUEST FOR ACKS ]

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

Thanks!

-- Steve

Quoted 146 lines

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 <[email protected]>
Cc Patrick McHardy <[email protected]>
Cc Jozsef Kadlecsik <[email protected]>
---
 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(-)
include/net/netfilter/nf_conntrack_l4proto.h +1 -1
index 4c8d573830b7..82e4ec002a39 100644
@@ -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);
net/ipv4/netfilter/nf_conntrack_l3proto_ipv4_compat.c +4 -1
index 4c48e434bb1f..91f207c2cb69 100644
@@ -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,
net/netfilter/nf_conntrack_proto_dccp.c +2 -2
index cb372f96f10d..15971177470a 100644
@@ -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)
net/netfilter/nf_conntrack_proto_gre.c +4 -4
index d5665739e3b1..cba607ada069 100644
@@ -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)
net/netfilter/nf_conntrack_proto_sctp.c +2 -2
index 1314d33f6bcf..c61f4cd6407d 100644
@@ -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)	\
net/netfilter/nf_conntrack_proto_tcp.c +2 -2
index 44d1ea32570a..79668fd3db96 100644
@@ -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)
net/netfilter/nf_conntrack_standalone.c +2 -2
index cf65a1e040dd..348aa3602787 100644
@@ -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))
#15 lore raw

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

Quoted 9 lines · On Tue, 4 Nov 2014 08:05:35 -0500 Steven Rostedt <[email protected]> wrote:
Quoted 3 lines · On Wed, 29 Oct 2014 17:56:04 -0400 Steven Rostedt <[email protected]> wrote:

From: "Steven Rostedt (Red Hat)" <[email protected]>

[ 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:

Quoted 1 line
Quoted 1 line
Quoted 1 line

[email protected] (after RCPT TO): 550 5.7.1 <[email protected]>: 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

maintainer Acked-by
#16 lore raw

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

In reply to Steven Rostedt
Quoted 8 lines · Steven Rostedt
Quoted 3 lines · On Wed, 29 Oct 2014 17:56:04 -0400 Steven Rostedt <[email protected]> wrote:

From: "Steven Rostedt (Red Hat)" <[email protected]>

[ REQUEST FOR ACKS ]

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

If Florian's concern were addressed, then:

to this patch and 4/8.

#17 lore raw

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

Quoted 11 lines · On Tue, 4 Nov 2014 15:22:36 +0100 Pablo Neira Ayuso <[email protected]> wrote:
Quoted 8 lines · Steven Rostedt
Quoted 3 lines · On Wed, 29 Oct 2014 17:56:04 -0400 Steven Rostedt <[email protected]> wrote:

From: "Steven Rostedt (Red Hat)" <[email protected]>

[ 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.

Quoted 4 lines

to this patch and 4/8.

Thanks for this!

Can I get someone to ack 3/8?

Thanks again,

-- Steve

#18 lore raw

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

Quoted 21 lines · Steven Rostedt
Quoted 11 lines · On Tue, 4 Nov 2014 15:22:36 +0100 Pablo Neira Ayuso <[email protected]> wrote:
Quoted 8 lines · Steven Rostedt
Quoted 3 lines · On Wed, 29 Oct 2014 17:56:04 -0400 Steven Rostedt <[email protected]> wrote:

From: "Steven Rostedt (Red Hat)" <[email protected]>

[ 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.

Quoted 1 line
Quoted 7 lines
Quoted 3 lines

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.

#19 lore raw

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

Quoted 5 lines · On Tue, 4 Nov 2014 15:46:32 +0100 Pablo Neira Ayuso <[email protected]> wrote:
Quoted 1 line

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

#20 lore raw

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

In reply to Pablo Neira Ayuso
Quoted 5 lines · On Tue, 4 Nov 2014 15:46:32 +0100 Pablo Neira Ayuso <[email protected]> wrote:
Quoted 5 lines
Quoted 4 lines

to this patch and 4/8.

Quoted 7 lines
Quoted 3 lines

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

#21 lore raw

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

In reply to Steven Rostedt
Quoted 4 lines · Steven Rostedt
Quoted 2 lines · On Tue, 4 Nov 2014 15:22:36 +0100 Pablo Neira Ayuso <[email protected]> wrote:
Quoted 1 line · Steven Rostedt

On Wed, 29 Oct 2014 17:56:04 -0400 Steven Rostedt <[email protected]> wrote:

Quoted 8 lines
Quoted 4 lines
Quoted 3 lines
Quoted 2 lines

From: "Steven Rostedt (Red Hat)" <[email protected]> [ 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.

#22 lore raw

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

Quoted 17 lines · On Tue, 04 Nov 2014 06:46:19 -0800 Joe Perches <[email protected]> wrote:
Quoted 4 lines · Steven Rostedt
Quoted 2 lines · On Tue, 4 Nov 2014 15:22:36 +0100 Pablo Neira Ayuso <[email protected]> wrote:
Quoted 1 line · Steven Rostedt

On Wed, 29 Oct 2014 17:56:04 -0400 Steven Rostedt <[email protected]> wrote:

Quoted 8 lines
Quoted 4 lines
Quoted 3 lines
Quoted 2 lines

From: "Steven Rostedt (Red Hat)" <[email protected]> [ 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.

Quoted 7 lines

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

#23 lore raw

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

In reply to Steven Rostedt

From: Joe Perches <[email protected]>

[ 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.

---
 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(-)
include/net/netfilter/nf_conntrack_core.h +1 -1
index cc0c18827602..f2f0fa3bb150 100644
@@ -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);
include/net/netfilter/nf_conntrack_l3proto.h +2 -2
index adc1fa3dd7ab..cdc920b4c4c2 100644
@@ -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. 
include/net/netfilter/nf_conntrack_l4proto.h +2 -2
index 82e4ec002a39..1f7061313d54 100644
@@ -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 *);
net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c +3 -3
index a054fe083431..5c61328b7704 100644
@@ -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,
net/ipv4/netfilter/nf_conntrack_l3proto_ipv4_compat.c +8 -4
index 91f207c2cb69..d927f9e72130 100644
@@ -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))
net/ipv4/netfilter/nf_conntrack_proto_icmp.c +5 -5
index b91b2641adda..80d5554b9a88 100644
@@ -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)
net/ipv6/netfilter/nf_conntrack_l3proto_ipv6.c +3 -3
index 4cbc6b290dd5..b68d0e59c1f8 100644
@@ -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,
net/ipv6/netfilter/nf_conntrack_proto_icmpv6.c +5 -5
index b3807c5cb888..90388d606483 100644
@@ -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)
net/netfilter/nf_conntrack_l3proto_generic.c +2 -3
index e7eb807fe07d..cf9ace70bece 100644
@@ -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,
net/netfilter/nf_conntrack_proto_dccp.c +5 -5
index 15971177470a..6dd995c7c72b 100644
@@ -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)
net/netfilter/nf_conntrack_proto_generic.c +2 -3
index 957c1db66652..60865f110309 100644
@@ -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)
net/netfilter/nf_conntrack_proto_gre.c +5 -5
index cba607ada069..7648674f29c3 100644
@@ -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 */
net/netfilter/nf_conntrack_proto_sctp.c +5 -5
index c61f4cd6407d..b45da90fad32 100644
@@ -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. */
net/netfilter/nf_conntrack_proto_tcp.c +5 -5
index 79668fd3db96..36a3ac8ee9f5 100644
@@ -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. */
net/netfilter/nf_conntrack_proto_udp.c +5 -5
index 9d7721cbce4b..6957281ffee5 100644
@@ -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)
net/netfilter/nf_conntrack_proto_udplite.c +5 -5
index 2750e6c69f82..c5903d1649f9 100644
@@ -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)
net/netfilter/nf_conntrack_standalone.c +7 -8
index 348aa3602787..23a0dcab21d4 100644
@@ -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;
Signature · 1 line
2.1.1
#24 lore raw

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

Quoted 3 lines · On Wed, 29 Oct 2014 17:56:05 -0400 Steven Rostedt <[email protected]> wrote:

From: Joe Perches <[email protected]>

[ REQUEST FOR ACKS ]

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

Thanks!

-- Steve

Quoted 402 lines

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.

---
 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(-)
include/net/netfilter/nf_conntrack_core.h +1 -1
index cc0c18827602..f2f0fa3bb150 100644
@@ -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);
include/net/netfilter/nf_conntrack_l3proto.h +2 -2
index adc1fa3dd7ab..cdc920b4c4c2 100644
@@ -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. 
include/net/netfilter/nf_conntrack_l4proto.h +2 -2
index 82e4ec002a39..1f7061313d54 100644
@@ -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 *);
net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c +3 -3
index a054fe083431..5c61328b7704 100644
@@ -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,
net/ipv4/netfilter/nf_conntrack_l3proto_ipv4_compat.c +8 -4
index 91f207c2cb69..d927f9e72130 100644
@@ -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))
net/ipv4/netfilter/nf_conntrack_proto_icmp.c +5 -5
index b91b2641adda..80d5554b9a88 100644
@@ -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)
net/ipv6/netfilter/nf_conntrack_l3proto_ipv6.c +3 -3
index 4cbc6b290dd5..b68d0e59c1f8 100644
@@ -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,
net/ipv6/netfilter/nf_conntrack_proto_icmpv6.c +5 -5
index b3807c5cb888..90388d606483 100644
@@ -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)
net/netfilter/nf_conntrack_l3proto_generic.c +2 -3
index e7eb807fe07d..cf9ace70bece 100644
@@ -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,
net/netfilter/nf_conntrack_proto_dccp.c +5 -5
index 15971177470a..6dd995c7c72b 100644
@@ -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)
net/netfilter/nf_conntrack_proto_generic.c +2 -3
index 957c1db66652..60865f110309 100644
@@ -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)
net/netfilter/nf_conntrack_proto_gre.c +5 -5
index cba607ada069..7648674f29c3 100644
@@ -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 */
net/netfilter/nf_conntrack_proto_sctp.c +5 -5
index c61f4cd6407d..b45da90fad32 100644
@@ -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. */
net/netfilter/nf_conntrack_proto_tcp.c +5 -5
index 79668fd3db96..36a3ac8ee9f5 100644
@@ -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. */
net/netfilter/nf_conntrack_proto_udp.c +5 -5
index 9d7721cbce4b..6957281ffee5 100644
@@ -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)
net/netfilter/nf_conntrack_proto_udplite.c +5 -5
index 2750e6c69f82..c5903d1649f9 100644
@@ -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)
net/netfilter/nf_conntrack_standalone.c +7 -8
index 348aa3602787..23a0dcab21d4 100644
@@ -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;
#25 lore raw

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

In reply to Steven Rostedt

Pablo,

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

Thanks,

-- Steve

Quoted 405 lines · On Wed, 29 Oct 2014 17:56:05 -0400 Steven Rostedt <[email protected]> wrote:

From: Joe Perches <[email protected]>

[ 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.

---
 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(-)
include/net/netfilter/nf_conntrack_core.h +1 -1
index cc0c18827602..f2f0fa3bb150 100644
@@ -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);
include/net/netfilter/nf_conntrack_l3proto.h +2 -2
index adc1fa3dd7ab..cdc920b4c4c2 100644
@@ -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. 
include/net/netfilter/nf_conntrack_l4proto.h +2 -2
index 82e4ec002a39..1f7061313d54 100644
@@ -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 *);
net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c +3 -3
index a054fe083431..5c61328b7704 100644
@@ -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,
net/ipv4/netfilter/nf_conntrack_l3proto_ipv4_compat.c +8 -4
index 91f207c2cb69..d927f9e72130 100644
@@ -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))
net/ipv4/netfilter/nf_conntrack_proto_icmp.c +5 -5
index b91b2641adda..80d5554b9a88 100644
@@ -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)
net/ipv6/netfilter/nf_conntrack_l3proto_ipv6.c +3 -3
index 4cbc6b290dd5..b68d0e59c1f8 100644
@@ -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,
net/ipv6/netfilter/nf_conntrack_proto_icmpv6.c +5 -5
index b3807c5cb888..90388d606483 100644
@@ -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)
net/netfilter/nf_conntrack_l3proto_generic.c +2 -3
index e7eb807fe07d..cf9ace70bece 100644
@@ -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,
net/netfilter/nf_conntrack_proto_dccp.c +5 -5
index 15971177470a..6dd995c7c72b 100644
@@ -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)
net/netfilter/nf_conntrack_proto_generic.c +2 -3
index 957c1db66652..60865f110309 100644
@@ -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)
net/netfilter/nf_conntrack_proto_gre.c +5 -5
index cba607ada069..7648674f29c3 100644
@@ -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 */
net/netfilter/nf_conntrack_proto_sctp.c +5 -5
index c61f4cd6407d..b45da90fad32 100644
@@ -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. */
net/netfilter/nf_conntrack_proto_tcp.c +5 -5
index 79668fd3db96..36a3ac8ee9f5 100644
@@ -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. */
net/netfilter/nf_conntrack_proto_udp.c +5 -5
index 9d7721cbce4b..6957281ffee5 100644
@@ -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)
net/netfilter/nf_conntrack_proto_udplite.c +5 -5
index 2750e6c69f82..c5903d1649f9 100644
@@ -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)
net/netfilter/nf_conntrack_standalone.c +7 -8
index 348aa3602787..23a0dcab21d4 100644
@@ -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;
#26 lore raw

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

In reply to Steven Rostedt

From: "Steven Rostedt (Red Hat)" <[email protected]>

[ 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 <[email protected]>
Cc Patrick McHardy <[email protected]>
Cc Jozsef Kadlecsik <[email protected]>
---
 .../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(-)
net/ipv4/netfilter/nf_conntrack_l3proto_ipv4_compat.c +16 -20
index d927f9e72130..a460a87e14f8 100644
@@ -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);
net/netfilter/nf_conntrack_standalone.c +29 -31

124 lines not on this page · raw message

net/netfilter/nf_log.c +16 -14
index d7197649dba6..6e3b9117db1f 100644
@@ -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 = {
net/netfilter/nfnetlink_queue_core.c +7 -6
index a82077d9f59b..f823f1538c4f 100644
@@ -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 = {
net/netfilter/x_tables.c +12 -7
index 133eb4772f12..51a459c3c649 100644
@@ -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;
 }
net/netfilter/xt_hashlimit.c +17 -19
index 05fbc2a0be46..178696852bde 100644
@@ -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)
Signature · 1 line
2.1.1
#27 lore raw

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

Quoted 3 lines · On Wed, 29 Oct 2014 17:56:06 -0400 Steven Rostedt <[email protected]> wrote:

From: "Steven Rostedt (Red Hat)" <[email protected]>

[ REQUEST FOR ACKS ]

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

Thanks!

-- Steve

Quoted 429 lines

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 <[email protected]>
Cc Patrick McHardy <[email protected]>
Cc Jozsef Kadlecsik <[email protected]>
---
 .../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(-)
net/ipv4/netfilter/nf_conntrack_l3proto_ipv4_compat.c +16 -20
index d927f9e72130..a460a87e14f8 100644
@@ -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);
net/netfilter/nf_conntrack_standalone.c +29 -31
index 23a0dcab21d4..fc823fa5dcf5 100644
@@ -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;
net/netfilter/nf_log.c +16 -14
index d7197649dba6..6e3b9117db1f 100644
@@ -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 = {
net/netfilter/nfnetlink_queue_core.c +7 -6
index a82077d9f59b..f823f1538c4f 100644
@@ -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 = {
net/netfilter/x_tables.c +12 -7
index 133eb4772f12..51a459c3c649 100644
@@ -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;
 }
net/netfilter/xt_hashlimit.c +17 -19
index 05fbc2a0be46..178696852bde 100644
@@ -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)
#28 lore raw

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

In reply to Steven Rostedt

From: Joe Perches <[email protected]>

[ 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.

Cc Christine Caulfield <[email protected]>
Cc David Teigland <[email protected]>
Cc cluster-devel at redhat.com
---
 fs/dlm/debug_fs.c | 251 +++++++++++++++++++++++++-----------------------------
 1 file changed, 117 insertions(+), 134 deletions(-)
fs/dlm/debug_fs.c +117 -134
index 1323c568e362..3bf460894088 100644
@@ -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;
Signature · 1 line
2.1.1
#29 lore raw

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

Quoted 3 lines · On Wed, 29 Oct 2014 17:56:07 -0400 Steven Rostedt <[email protected]> wrote:

From: Joe Perches <[email protected]>

[ REQUEST FOR ACKS ]

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

Thanks!

-- Steve

Quoted 484 lines

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.

Cc Christine Caulfield <[email protected]>
Cc David Teigland <[email protected]>
Cc cluster-devel at redhat.com
---
 fs/dlm/debug_fs.c | 251 +++++++++++++++++++++++++-----------------------------
 1 file changed, 117 insertions(+), 134 deletions(-)
fs/dlm/debug_fs.c +117 -134
index 1323c568e362..3bf460894088 100644
@@ -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;
maintainer
#30 lore raw

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

Quoted 8 lines · Steven Rostedt
Quoted 3 lines · On Wed, 29 Oct 2014 17:56:07 -0400 Steven Rostedt <[email protected]> wrote:

From: Joe Perches <[email protected]>

[ REQUEST FOR ACKS ]

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

Looks ok,
Dave

#31 lore raw

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

In reply to Steven Rostedt

From: Joe Perches <[email protected]>

[ REQUEST FOR ACKS ]

Convert the seq_printf output with constant strings to seq_puts.

Cc Christine Caulfield <[email protected]>
Cc David Teigland <[email protected]>
Cc cluster-devel at redhat.com
---
 fs/dlm/debug_fs.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)
fs/dlm/debug_fs.c +5 -7
index 3bf460894088..eea64912c9c0 100644
@@ -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);
Signature · 1 line
2.1.1
#32 lore raw

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

Quoted 3 lines · On Wed, 29 Oct 2014 17:56:08 -0400 Steven Rostedt <[email protected]> wrote:

From: Joe Perches <[email protected]>

[ REQUEST FOR ACKS ]

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

Thanks!

-- Steve

Quoted 63 lines

Convert the seq_printf output with constant strings to seq_puts.

Cc Christine Caulfield <[email protected]>
Cc David Teigland <[email protected]>
Cc cluster-devel at redhat.com
---
 fs/dlm/debug_fs.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)
fs/dlm/debug_fs.c +5 -7
index 3bf460894088..eea64912c9c0 100644
@@ -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);
#33 lore raw

PATCH 7/8 fs: Convert show_fdinfo functions to void

In reply to Steven Rostedt

From: Joe Perches <[email protected]>

[ REQUEST FOR ACKS ]

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

Update vfs documentation.

Cc David S. Miller <[email protected]>
Cc Al Viro <[email protected]>
[ did a few clean ups ]
---
 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(-)
Documentation/filesystems/vfs.txt +1 -1
index fceff7c00a3c..af1dbc1823bb 100644
@@ -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
drivers/net/tun.c +2 -2
index 186ce541c657..a3420e091689 100644
@@ -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
 
fs/eventfd.c +3 -6
index d6a88e7812f3..4b0a226024fa 100644
@@ -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
 
fs/eventpoll.c +5 -8
index 7bcfff900f05..d77f94491352 100644
@@ -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
 
fs/notify/fdinfo.c +32 -46

161 lines not on this page · raw message

fs/notify/fdinfo.h +2 -2
index 556afda990e9..9664c4904d6b 100644
@@ -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 */
fs/proc/fd.c +2 -1
index e11d7c590bb0..8e5ad83b629a 100644
@@ -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);
 	}
 
fs/signalfd.c +1 -3
index 424b7b65321f..7e412ad74836 100644
@@ -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
 
fs/timerfd.c +14 -13
index b46ffa94372a..b94fa6c3c6eb 100644
@@ -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
include/linux/fs.h +1 -1
index a957d4366c24..01dd9052a142 100644
@@ -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 {
Signature · 1 line
2.1.1
#34 lore raw

PATCH 8/8 debugfs: Have debugfs_print_regs32() return void

In reply to Steven Rostedt

From: Joe Perches <[email protected]>

[ 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.

Cc Greg Kroah-Hartman <[email protected]>

[ original change only updated seq_printf() return, added return of
void to debugfs_print_regs32() as well ]

---
 Documentation/filesystems/debugfs.txt |  2 +-
 fs/debugfs/file.c                     | 15 ++++++++-------
 include/linux/debugfs.h               |  7 +++----
 3 files changed, 12 insertions(+), 12 deletions(-)
Documentation/filesystems/debugfs.txt +1 -1
index 3a863f692728..88ab81c79109 100644
@@ -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
fs/debugfs/file.c +8 -7
index 76c08c2beb2f..8e0f2f410189 100644
@@ -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);
 
include/linux/debugfs.h +3 -4
index 4d0b4d1aa132..d84f8c254a87 100644
@@ -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)
Signature · 1 line
2.1.1
maintainer Acked-by on v1 8/8
#35 lore raw

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

Quoted 12 lines · Steven Rostedt

From: Joe Perches <[email protected]>

[ 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.

Quoted 8 lines
Cc Greg Kroah-Hartman <[email protected]>

[ original change only updated seq_printf() return, added return of
void to debugfs_print_regs32() as well ]

---

#36 lore raw

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

Quoted 15 lines · On Wed, 29 Oct 2014 15:03:15 -0700 Greg Kroah-Hartman…
Quoted 14 lines · Steven Rostedt

From: Joe Perches <[email protected]>

[ 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.

Quoted 8 lines
Quoted 6 lines
Cc Greg Kroah-Hartman <[email protected]>

[ original change only updated seq_printf() return, added return of
void to debugfs_print_regs32() as well ]

---

Thanks!

-- Steve

#37 lore raw
In reply to Steven Rostedt
Quoted 1 line · On Wed, 29 Oct 2014 17:56:02 -0400 Steven Rostedt <[email protected]> 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

Quoted 80 lines

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(-)
maintainer
#38 lore raw
Quoted 9 lines · Steven Rostedt
Quoted 1 line · On Wed, 29 Oct 2014 17:56:02 -0400 Steven Rostedt <[email protected]> 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?

#39 lore raw
Quoted 4 lines · On Wed, 5 Nov 2014 17:50:21 +0000 Al Viro <[email protected]> wrote:
Quoted 3 lines · Steven Rostedt

On Wed, 29 Oct 2014 17:56:02 -0400
Steven Rostedt <[email protected]> wrote:

Quoted 8 lines
Quoted 6 lines
Quoted 1 line

[ 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