Skip to contentSkip to conversation

ftrace: clear bits properly in reset_iter_read()

PATCH In review Dan Carpenter 2 participants 4 messages Jun 9, 2012 – Dec 9, 2012

Subsystems (3)
Kernel janitors Tracing ftrace
lore
Share & tools
lore

Dan Carpenter

author
#0 lore raw

PATCH 1/1 ftrace: clear bits properly in reset_iter_read()

There is a typo here where '&' is used instead of '|' and it turns the statement into a noop. The original code is equivalent to:

	iter->flags &= ~((1 << 2) & (1 << 4));
Signed-off-by Dan Carpenter <[email protected]>

---
This is a static checker fix and I'm not super familiar with ftrace. Please review carefully.

kernel/trace/ftrace.c +1 -1
index a008663..97da2dc 100644
@@ -2358,7 +2358,7 @@ static void reset_iter_read(struct ftrace_iterator *iter)
 {
 	iter->pos = 0;
 	iter->func_pos = 0;
-	iter->flags &= ~(FTRACE_ITER_PRINTALL & FTRACE_ITER_HASH);
+	iter->flags &= ~(FTRACE_ITER_PRINTALL | FTRACE_ITER_HASH);
 }
 
 static void *t_start(struct seq_file *m, loff_t *pos)
maintainer
#1 lore raw
Quoted 15 lines · Dan Carpenter

There is a typo here where '&' is used instead of '|' and it turns the statement into a noop. The original code is equivalent to:

	iter->flags &= ~((1 << 2) & (1 << 4));
Signed-off-by Dan Carpenter <[email protected]>

---
This is a static checker fix and I'm not super familiar with ftrace. Please review carefully.

kernel/trace/ftrace.c +0 -0
index a008663..97da2dc 100644

kernel/trace/ftrace.c static void reset_iter_read(struct ftrace_iterator *iter) · line 2361

Quoted 5 lines
kernel/trace/ftrace.c +1 -1
@@ -2358,7 +2358,7 @@ static void reset_iter_read(struct ftrace_iterator *iter)
 {
 	iter->pos = 0;
 	iter->func_pos = 0;
-	iter->flags &= ~(FTRACE_ITER_PRINTALL & FTRACE_ITER_HASH);
+	iter->flags &= ~(FTRACE_ITER_PRINTALL | FTRACE_ITER_HASH);

Thanks! This is a real minor bug, but as it is a simple fix I will probably queue it up for 3.5 and stable. The reason that it has gone unnoticed for so long is that this would only show up if you did a lseek on the function list file. Which is not a common operation to do.

I checked the code and if someone were to do an lseek with these flags set then they would just get the hash list again (and not the function list). It's a bug, yes, but not a big one.

I'll queue it up on Monday.

-- Steve

Quoted 3 lines
kernel/trace/ftrace.c +0 -0
 }
 
 static void *t_start(struct seq_file *m, loff_t *pos)
maintainer
#2 lore raw

(Doing my slow clean ups, I find lots of coins under the couch cushions)

Quoted 27 lines · Steven Rostedt
Quoted 15 lines · Dan Carpenter

There is a typo here where '&' is used instead of '|' and it turns the statement into a noop. The original code is equivalent to:

	iter->flags &= ~((1 << 2) & (1 << 4));
Signed-off-by Dan Carpenter <[email protected]>

---
This is a static checker fix and I'm not super familiar with ftrace. Please review carefully.

kernel/trace/ftrace.c +0 -0
index a008663..97da2dc 100644

kernel/trace/ftrace.c static void reset_iter_read(struct ftrace_iterator *iter) · line 2361

Quoted 5 lines
kernel/trace/ftrace.c +1 -1
@@ -2358,7 +2358,7 @@ static void reset_iter_read(struct ftrace_iterator *iter)
 {
 	iter->pos = 0;
 	iter->func_pos = 0;
-	iter->flags &= ~(FTRACE_ITER_PRINTALL & FTRACE_ITER_HASH);
+	iter->flags &= ~(FTRACE_ITER_PRINTALL | FTRACE_ITER_HASH);

Thanks! This is a real minor bug, but as it is a simple fix I will probably queue it up for 3.5 and stable. The reason that it has gone unnoticed for so long is that this would only show up if you did a lseek on the function list file. Which is not a common operation to do.

Quoted 6 lines

I checked the code and if someone were to do an lseek with these flags set then they would just get the hash list again (and not the function list). It's a bug, yes, but not a big one.

I'll queue it up on Monday.

I just didn't say *which* Monday :-p

Bah, this got lost in a rebase somehow. Grumble, I should push this out now as urgent.

-- Steve

Quoted 5 lines
Quoted 4 lines
kernel/trace/ftrace.c +0 -0
 }
 
 static void *t_start(struct seq_file *m, loff_t *pos)
author
#3 lore raw

[tip:perf/urgent] ftrace: Clear bits properly in reset_iter_read( )

In reply to Dan Carpenter
Commit-ID:  70f77b3f7ec010ff9624c1f2e39a81babc9e2429
Gitweb:     http://git.kernel.org/tip/70f77b3f7ec010ff9624c1f2e39a81babc9e2429
Author:     Dan Carpenter <[email protected]>
AuthorDate: Sat, 9 Jun 2012 19:10:27 +0300
Committer:  Steven Rostedt <[email protected]>
CommitDate: Thu, 15 Nov 2012 16:10:17 -0500

ftrace: Clear bits properly in reset_iter_read()

There is a typo here where '&' is used instead of '|' and it turns the statement into a noop. The original code is equivalent to:

	iter->flags &= ~((1 << 2) & (1 << 4));
Link http://lkml.kernel.org/r/[email protected] open here
Cc [email protected] # all of them
Signed-off-by Dan Carpenter <[email protected]>
Signed-off-by Steven Rostedt <[email protected]>
---
 kernel/trace/ftrace.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
kernel/trace/ftrace.c +1 -1
index 9dcf15d..51b7159 100644
@@ -2437,7 +2437,7 @@ static void reset_iter_read(struct ftrace_iterator *iter)
 {
 	iter->pos = 0;
 	iter->func_pos = 0;
-	iter->flags &= ~(FTRACE_ITER_PRINTALL & FTRACE_ITER_HASH);
+	iter->flags &= ~(FTRACE_ITER_PRINTALL | FTRACE_ITER_HASH);
 }
 
 static void *t_start(struct seq_file *m, loff_t *pos)