* [patch] trace: find the correct ftrace event
@ 2010-03-20 13:19 Dan Carpenter
2010-03-20 13:37 ` Steven Rostedt
0 siblings, 1 reply; 8+ messages in thread
From: Dan Carpenter @ 2010-03-20 13:19 UTC (permalink / raw)
To: Ingo Molnar
Cc: Frederic Weisbecker, Steven Rostedt, Li Zefan, Tom Zanussi,
linux-kernel, kernel-janitors
The original code doesn't work because "call" is never NULL there.
Signed-off-by: Dan Carpenter <error27@gmail.com>
---
My code should work, but it seems like there should but it seems like
there should be a more elegant way to do this?
diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
index 4615f62..6070c70 100644
--- a/kernel/trace/trace_events_filter.c
+++ b/kernel/trace/trace_events_filter.c
@@ -1388,16 +1388,19 @@ int ftrace_profile_set_filter(struct perf_event *event, int event_id,
struct event_filter *filter;
struct filter_parse_state *ps;
struct ftrace_event_call *call = NULL;
+ int found = 0;
mutex_lock(&event_mutex);
list_for_each_entry(call, &ftrace_events, list) {
- if (call->id == event_id)
+ if (call->id == event_id) {
+ found = 1;
break;
+ }
}
err = -EINVAL;
- if (!call)
+ if (!found)
goto out_unlock;
err = -EEXIST;
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [patch] trace: find the correct ftrace event
2010-03-20 13:19 [patch] trace: find the correct ftrace event Dan Carpenter
@ 2010-03-20 13:37 ` Steven Rostedt
2010-03-20 13:56 ` Dan Carpenter
0 siblings, 1 reply; 8+ messages in thread
From: Steven Rostedt @ 2010-03-20 13:37 UTC (permalink / raw)
To: Dan Carpenter
Cc: Ingo Molnar, Frederic Weisbecker, Li Zefan, Tom Zanussi,
linux-kernel, kernel-janitors
On Sat, 2010-03-20 at 16:19 +0300, Dan Carpenter wrote:
> The original code doesn't work because "call" is never NULL there.
A one line change would have done the same ;-)
> Signed-off-by: Dan Carpenter <error27@gmail.com>
> ---
> My code should work, but it seems like there should but it seems like
> there should be a more elegant way to do this?
>
> diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
> index 4615f62..6070c70 100644
> --- a/kernel/trace/trace_events_filter.c
> +++ b/kernel/trace/trace_events_filter.c
> @@ -1388,16 +1388,19 @@ int ftrace_profile_set_filter(struct perf_event *event, int event_id,
> struct event_filter *filter;
> struct filter_parse_state *ps;
> struct ftrace_event_call *call = NULL;
> + int found = 0;
>
> mutex_lock(&event_mutex);
>
> list_for_each_entry(call, &ftrace_events, list) {
> - if (call->id == event_id)
> + if (call->id == event_id) {
> + found = 1;
> break;
> + }
> }
>
> err = -EINVAL;
> - if (!call)
if (call == &ftrace_events)
-- Steve
> + if (!found)
> goto out_unlock;
>
> err = -EEXIST;
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [patch] trace: find the correct ftrace event
2010-03-20 13:37 ` Steven Rostedt
@ 2010-03-20 13:56 ` Dan Carpenter
2010-03-20 14:07 ` Steven Rostedt
0 siblings, 1 reply; 8+ messages in thread
From: Dan Carpenter @ 2010-03-20 13:56 UTC (permalink / raw)
To: Steven Rostedt
Cc: Ingo Molnar, Frederic Weisbecker, Li Zefan, Tom Zanussi,
linux-kernel, kernel-janitors
On Sat, Mar 20, 2010 at 09:37:13AM -0400, Steven Rostedt wrote:
> On Sat, 2010-03-20 at 16:19 +0300, Dan Carpenter wrote:
> > The original code doesn't work because "call" is never NULL there.
>
> A one line change would have done the same ;-)
>
> > Signed-off-by: Dan Carpenter <error27@gmail.com>
> > ---
> > My code should work, but it seems like there should but it seems like
> > there should be a more elegant way to do this?
> >
> > diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
> > index 4615f62..6070c70 100644
> > --- a/kernel/trace/trace_events_filter.c
> > +++ b/kernel/trace/trace_events_filter.c
> > @@ -1388,16 +1388,19 @@ int ftrace_profile_set_filter(struct perf_event *event, int event_id,
> > struct event_filter *filter;
> > struct filter_parse_state *ps;
> > struct ftrace_event_call *call = NULL;
> > + int found = 0;
> >
> > mutex_lock(&event_mutex);
> >
> > list_for_each_entry(call, &ftrace_events, list) {
> > - if (call->id == event_id)
> > + if (call->id == event_id) {
> > + found = 1;
> > break;
> > + }
> > }
> >
> > err = -EINVAL;
> > - if (!call)
>
> if (call == &ftrace_events)
>
Man... That makes my head hurt. It only works because ->list is the
first element in the struct.
We'd need a cast.
Is that really cleaner?
regards,
dan carpenter
> -- Steve
>
> > + if (!found)
> > goto out_unlock;
> >
> > err = -EEXIST;
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [patch] trace: find the correct ftrace event
2010-03-20 13:56 ` Dan Carpenter
@ 2010-03-20 14:07 ` Steven Rostedt
2010-03-20 14:31 ` Dan Carpenter
2010-03-20 14:39 ` [patch v2] " Dan Carpenter
0 siblings, 2 replies; 8+ messages in thread
From: Steven Rostedt @ 2010-03-20 14:07 UTC (permalink / raw)
To: Dan Carpenter
Cc: Ingo Molnar, Frederic Weisbecker, Li Zefan, Tom Zanussi,
linux-kernel, kernel-janitors
On Sat, 2010-03-20 at 16:56 +0300, Dan Carpenter wrote:
> > > diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
> > > index 4615f62..6070c70 100644
> > > --- a/kernel/trace/trace_events_filter.c
> > > +++ b/kernel/trace/trace_events_filter.c
> > > @@ -1388,16 +1388,19 @@ int ftrace_profile_set_filter(struct perf_event *event, int event_id,
> > > struct event_filter *filter;
> > > struct filter_parse_state *ps;
> > > struct ftrace_event_call *call = NULL;
> > > + int found = 0;
> > >
> > > mutex_lock(&event_mutex);
> > >
> > > list_for_each_entry(call, &ftrace_events, list) {
> > > - if (call->id == event_id)
> > > + if (call->id == event_id) {
> > > + found = 1;
> > > break;
> > > + }
> > > }
> > >
> > > err = -EINVAL;
> > > - if (!call)
> >
> > if (call == &ftrace_events)
> >
>
> Man... That makes my head hurt. It only works because ->list is the
> first element in the struct.
>
> We'd need a cast.
Oops, I missed the fact that it was list_*_entry(). That should be:
if (call->list != &ftrace_events)
No cast needed.
>
> Is that really cleaner?
Perhaps not, but I think it is more elegant ;-)
-- Steve
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [patch] trace: find the correct ftrace event
2010-03-20 14:07 ` Steven Rostedt
@ 2010-03-20 14:31 ` Dan Carpenter
2010-03-20 14:39 ` [patch v2] " Dan Carpenter
1 sibling, 0 replies; 8+ messages in thread
From: Dan Carpenter @ 2010-03-20 14:31 UTC (permalink / raw)
To: Steven Rostedt
Cc: Ingo Molnar, Frederic Weisbecker, Li Zefan, Tom Zanussi,
linux-kernel, kernel-janitors
On Sat, Mar 20, 2010 at 10:07:16AM -0400, Steven Rostedt wrote:
> On Sat, 2010-03-20 at 16:56 +0300, Dan Carpenter wrote:
>
> > > > diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
> > > > index 4615f62..6070c70 100644
> > > > --- a/kernel/trace/trace_events_filter.c
> > > > +++ b/kernel/trace/trace_events_filter.c
> > > > @@ -1388,16 +1388,19 @@ int ftrace_profile_set_filter(struct perf_event *event, int event_id,
> > > > struct event_filter *filter;
> > > > struct filter_parse_state *ps;
> > > > struct ftrace_event_call *call = NULL;
> > > > + int found = 0;
> > > >
> > > > mutex_lock(&event_mutex);
> > > >
> > > > list_for_each_entry(call, &ftrace_events, list) {
> > > > - if (call->id == event_id)
> > > > + if (call->id == event_id) {
> > > > + found = 1;
> > > > break;
> > > > + }
> > > > }
> > > >
> > > > err = -EINVAL;
> > > > - if (!call)
> > >
> > > if (call == &ftrace_events)
> > >
> >
> > Man... That makes my head hurt. It only works because ->list is the
> > first element in the struct.
> >
> > We'd need a cast.
>
> Oops, I missed the fact that it was list_*_entry(). That should be:
>
> if (call->list != &ftrace_events)
>
> No cast needed.
>
Hm... "list" is a struct not a pointer to a struct.
So it would have to be:
if (&call->list == &ftrace_events)
goto out_unlock;
That's how the list_for_each_entry() determines the end of the list as
well.
I'll send a patch to do that.
regards,
dan carpenter
> >
> > Is that really cleaner?
>
> Perhaps not, but I think it is more elegant ;-)
>
> -- Steve
^ permalink raw reply [flat|nested] 8+ messages in thread* [patch v2] trace: find the correct ftrace event
2010-03-20 14:07 ` Steven Rostedt
2010-03-20 14:31 ` Dan Carpenter
@ 2010-03-20 14:39 ` Dan Carpenter
2010-03-21 1:09 ` Steven Rostedt
2010-05-07 18:40 ` [tip:perf/core] perf: Fix check at end of event search tip-bot for Dan Carpenter
1 sibling, 2 replies; 8+ messages in thread
From: Dan Carpenter @ 2010-03-20 14:39 UTC (permalink / raw)
To: Steven Rostedt
Cc: Ingo Molnar, Frederic Weisbecker, Li Zefan, Tom Zanussi,
linux-kernel, kernel-janitors
The original code doesn't work because "call" is never NULL there.
Signed-off-by: Dan Carpenter <error27@gmail.com>
---
The original patch was fine, but this version is more tasteful. ;)
diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
index 4615f62..c1a263a 100644
--- a/kernel/trace/trace_events_filter.c
+++ b/kernel/trace/trace_events_filter.c
@@ -1397,7 +1397,7 @@ int ftrace_profile_set_filter(struct perf_event *event, int event_id,
}
err = -EINVAL;
- if (!call)
+ if (&call->list == &ftrace_events)
goto out_unlock;
err = -EEXIST;
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [patch v2] trace: find the correct ftrace event
2010-03-20 14:39 ` [patch v2] " Dan Carpenter
@ 2010-03-21 1:09 ` Steven Rostedt
2010-05-07 18:40 ` [tip:perf/core] perf: Fix check at end of event search tip-bot for Dan Carpenter
1 sibling, 0 replies; 8+ messages in thread
From: Steven Rostedt @ 2010-03-21 1:09 UTC (permalink / raw)
To: Dan Carpenter
Cc: Ingo Molnar, Frederic Weisbecker, Li Zefan, Tom Zanussi,
linux-kernel, kernel-janitors
On Sat, 2010-03-20 at 17:39 +0300, Dan Carpenter wrote:
> The original code doesn't work because "call" is never NULL there.
>
> Signed-off-by: Dan Carpenter <error27@gmail.com>
Thanks, I'll apply and test this on Monday.
-- Steve
>
> ---
> The original patch was fine, but this version is more tasteful. ;)
>
> diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
> index 4615f62..c1a263a 100644
> --- a/kernel/trace/trace_events_filter.c
> +++ b/kernel/trace/trace_events_filter.c
> @@ -1397,7 +1397,7 @@ int ftrace_profile_set_filter(struct perf_event *event, int event_id,
> }
>
> err = -EINVAL;
> - if (!call)
> + if (&call->list == &ftrace_events)
> goto out_unlock;
>
> err = -EEXIST;
^ permalink raw reply [flat|nested] 8+ messages in thread
* [tip:perf/core] perf: Fix check at end of event search
2010-03-20 14:39 ` [patch v2] " Dan Carpenter
2010-03-21 1:09 ` Steven Rostedt
@ 2010-05-07 18:40 ` tip-bot for Dan Carpenter
1 sibling, 0 replies; 8+ messages in thread
From: tip-bot for Dan Carpenter @ 2010-05-07 18:40 UTC (permalink / raw)
To: linux-tip-commits; +Cc: linux-kernel, hpa, mingo, rostedt, error27, tglx
Commit-ID: d9f599e1e6d019968b35d2dc63074b9e8964fa69
Gitweb: http://git.kernel.org/tip/d9f599e1e6d019968b35d2dc63074b9e8964fa69
Author: Dan Carpenter <error27@gmail.com>
AuthorDate: Sat, 20 Mar 2010 17:39:11 +0300
Committer: Steven Rostedt <rostedt@goodmis.org>
CommitDate: Thu, 6 May 2010 19:49:52 -0400
perf: Fix check at end of event search
The original code doesn't work because "call" is never NULL there.
Signed-off-by: Dan Carpenter <error27@gmail.com>
LKML-Reference: <20100320143911.GF5331@bicker>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
kernel/trace/trace_events_filter.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
index 88c0b6d..58092d8 100644
--- a/kernel/trace/trace_events_filter.c
+++ b/kernel/trace/trace_events_filter.c
@@ -1398,7 +1398,7 @@ int ftrace_profile_set_filter(struct perf_event *event, int event_id,
}
err = -EINVAL;
- if (!call)
+ if (&call->list == &ftrace_events)
goto out_unlock;
err = -EEXIST;
^ permalink raw reply related [flat|nested] 8+ messages in thread
end of thread, other threads:[~2010-05-07 18:41 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-03-20 13:19 [patch] trace: find the correct ftrace event Dan Carpenter
2010-03-20 13:37 ` Steven Rostedt
2010-03-20 13:56 ` Dan Carpenter
2010-03-20 14:07 ` Steven Rostedt
2010-03-20 14:31 ` Dan Carpenter
2010-03-20 14:39 ` [patch v2] " Dan Carpenter
2010-03-21 1:09 ` Steven Rostedt
2010-05-07 18:40 ` [tip:perf/core] perf: Fix check at end of event search tip-bot for Dan Carpenter
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.