Skip to contentSkip to conversation

tracing: Remove an unneeded check

PATCH In review Dan Carpenter 2 participants 5 messages Apr 20, 2012 – May 2, 2012

Subsystems (2)
Kernel janitors Tracing
lore
Share & tools
lore

Dan Carpenter

author
#0 lore raw

PATCH 1/1 tracing: Remove an unneeded check

memcpy() returns a pointer to "bug". Hopefully, it's not NULL here or we would already have Oopsed.

Signed-off-by Dan Carpenter <[email protected]>
kernel/trace/trace.c +1 -4
index ed7b5d1..c2d5b71 100644
@@ -629,7 +629,6 @@ ssize_t trace_seq_to_user(struct trace_seq *s, char __user *ubuf, size_t cnt)
 static ssize_t trace_seq_to_buffer(struct trace_seq *s, void *buf, size_t cnt)
 {
 	int len;
-	void *ret;
 
 	if (s->len <= s->readpos)
 		return -EBUSY;

@@ -637,9 +636,7 @@ static ssize_t trace_seq_to_buffer(struct trace_seq *s, void *buf, size_t cnt)
 	len = s->len - s->readpos;
 	if (cnt > len)
 		cnt = len;
-	ret = memcpy(buf, s->buffer + s->readpos, cnt);
-	if (!ret)
-		return -EFAULT;
+	memcpy(buf, s->buffer + s->readpos, cnt);
 
 	s->readpos += cnt;
 	return cnt;
maintainer
#1 lore raw
Quoted 2 lines · Dan Carpenter

memcpy() returns a pointer to "bug". Hopefully, it's not NULL here or we would already have Oopsed.

You're right that the ret is useless, and the real bug was already fixed (the possibility that buf could be an invalid pointer).

Not only is this needed, but also get rid of the ret < 0 check in tracing_splice_read_pipe().

-- Steve

author
#2 lore raw
Quoted 10 lines · Steven Rostedt
Quoted 2 lines · Dan Carpenter

memcpy() returns a pointer to "bug". Hopefully, it's not NULL here or we would already have Oopsed.

You're right that the ret is useless, and the real bug was already fixed (the possibility that buf could be an invalid pointer).

Not only is this needed, but also get rid of the ret < 0 check in tracing_splice_read_pipe().

We still return -EBUSY so we need the check.

regards,
dan carpenter

maintainer
#3 lore raw
Quoted 5 lines · Dan Carpenter
Quoted 4 lines · Steven Rostedt
Quoted 2 lines · Dan Carpenter

memcpy() returns a pointer to "bug". Hopefully, it's not NULL here or we would already have Oopsed.

Quoted 8 lines
Quoted 6 lines

You're right that the ret is useless, and the real bug was already fixed (the possibility that buf could be an invalid pointer).

Not only is this needed, but also get rid of the ret < 0 check in tracing_splice_read_pipe().

We still return -EBUSY so we need the check.

Hmm, you're right. I need to take a deeper look at this code. It looks like it can be cleaned up a bit more (the splice code).

OK, your patch is good as is. I'll add it in my 3.5 queue, and add a mental note to look at this code again.

Thanks,

-- Steve

author
#4 lore raw

[tip:perf/core] tracing: Remove an unneeded check in trace_seq_buffer()

In reply to Dan Carpenter
Commit-ID:  5a26c8f0cf1e95106858bb4e23ca6dd14c9b842f
Gitweb:     http://git.kernel.org/tip/5a26c8f0cf1e95106858bb4e23ca6dd14c9b842f
Author:     Dan Carpenter <[email protected]>
AuthorDate: Fri, 20 Apr 2012 09:31:45 +0300
Committer:  Steven Rostedt <[email protected]>
CommitDate: Mon, 23 Apr 2012 21:16:10 -0400

tracing: Remove an unneeded check in trace_seq_buffer()

memcpy() returns a pointer to "bug". Hopefully, it's not NULL here or we would already have Oopsed.

Link http://lkml.kernel.org/r/[email protected] open here
Cc Frederic Weisbecker <[email protected]>
Cc Eduard - Gabriel Munteanu <[email protected]>
Signed-off-by Dan Carpenter <[email protected]>
Signed-off-by Steven Rostedt <[email protected]>
---
 kernel/trace/trace.c |    5 +----
 1 files changed, 1 insertions(+), 4 deletions(-)
kernel/trace/trace.c +1 -4
index 1ab8e35..bbcde54 100644
@@ -629,7 +629,6 @@ ssize_t trace_seq_to_user(struct trace_seq *s, char __user *ubuf, size_t cnt)
 static ssize_t trace_seq_to_buffer(struct trace_seq *s, void *buf, size_t cnt)
 {
 	int len;
-	void *ret;
 
 	if (s->len <= s->readpos)
 		return -EBUSY;

@@ -637,9 +636,7 @@ static ssize_t trace_seq_to_buffer(struct trace_seq *s, void *buf, size_t cnt)
 	len = s->len - s->readpos;
 	if (cnt > len)
 		cnt = len;
-	ret = memcpy(buf, s->buffer + s->readpos, cnt);
-	if (!ret)
-		return -EFAULT;
+	memcpy(buf, s->buffer + s->readpos, cnt);
 
 	s->readpos += cnt;
 	return cnt;