2020-08-13 21:05:33

by Josef Bacik

Subject: [PATCH 6/6] sunrpc: rework proc handlers to take advantage of the new buffer

Now that we're allocating an extra slot for the NULL terminated string,
use scnprintf() and write directly into the buffer.

Signed-off-by: Josef Bacik <[email protected]>
---
net/sunrpc/sysctl.c | 10 ++--------
net/sunrpc/xprtrdma/svc_rdma.c | 16 ++--------------
2 files changed, 4 insertions(+), 22 deletions(-)

diff --git a/net/sunrpc/sysctl.c b/net/sunrpc/sysctl.c
index 999eee1ed61c..31ed530d9846 100644
--- a/net/sunrpc/sysctl.c
+++ b/net/sunrpc/sysctl.c
@@ -117,14 +117,8 @@ proc_dodebug(struct ctl_table *table, int write, void *buffer, size_t *lenp,
if (strcmp(table->procname, "rpc_debug") == 0)
rpc_show_tasks(&init_net);
} else {
- len = sprintf(tmpbuf, "0x%04x", *(unsigned int *) table->data);
- if (len > left)
- len = left;
- memcpy(buffer, tmpbuf, len);
- if ((left -= len) > 0) {
- *((char *)buffer + len) = '\n';
- left--;
- }
+ len = scnprintf(buffer, *lenp, "0x%04x\n", *(unsigned int *) table->data);
+ left -= len;
}

done:
diff --git a/net/sunrpc/xprtrdma/svc_rdma.c b/net/sunrpc/xprtrdma/svc_rdma.c
index 526da5d4710b..9b3a113598af 100644
--- a/net/sunrpc/xprtrdma/svc_rdma.c
+++ b/net/sunrpc/xprtrdma/svc_rdma.c
@@ -90,20 +90,8 @@ static int read_reset_stat(struct ctl_table *table, int write,
if (write)
atomic_set(stat, 0);
else {
- char str_buf[32];
- int len = snprintf(str_buf, 32, "%d\n", atomic_read(stat));
- if (len >= 32)
- return -EFAULT;
- len = strlen(str_buf);
- if (*ppos > len) {
- *lenp = 0;
- return 0;
- }
- len -= *ppos;
- if (len > *lenp)
- len = *lenp;
- if (len)
- memcpy(buffer, str_buf, len);
+ size_t len = scnprintf(buffer, *lenp, "%d\n",
+ atomic_read(stat));
*lenp = len;
*ppos += len;
}
--
2.24.1

2020-09-01 15:18:57

by Christoph Hellwig

Subject: Re: [PATCH 6/6] sunrpc: rework proc handlers to take advantage of the new buffer

On Thu, Aug 13, 2020 at 05:04:11PM -0400, Josef Bacik wrote:
> + len = scnprintf(buffer, *lenp, "0x%04x\n", *(unsigned int *) table->data);

This adds a really hard to read overlong long line.

Otherwise looks good:

Reviewed-by: Christoph Hellwig <[email protected]>