* [PATCH] tracing: Remove redundant variable ret
@ 2021-05-27 17:14 Hyeonggon Yoo
2021-05-29 2:22 ` Steven Rostedt
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Hyeonggon Yoo @ 2021-05-27 17:14 UTC (permalink / raw)
To: Steven Rostedt, Ingo Molnar; +Cc: linux-kernel
This variable saves return value of event_hist_trigger_func,
but it's never read. So it's redundant.
Signed-off-by: Hyeonggon Yoo <42.hyeyoo@gmail.com>
---
kernel/trace/trace_events_hist.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
index c1abd63f1d6c..414f2727d7a7 100644
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -5225,12 +5225,11 @@ static void unregister_field_var_hists(struct hist_trigger_data *hist_data)
struct trace_event_file *file;
unsigned int i;
char *cmd;
- int ret;
for (i = 0; i < hist_data->n_field_var_hists; i++) {
file = hist_data->field_var_hists[i]->hist_data->event_file;
cmd = hist_data->field_var_hists[i]->cmd;
- ret = event_hist_trigger_func(&trigger_hist_cmd, file,
+ event_hist_trigger_func(&trigger_hist_cmd, file,
"!hist", "hist", cmd);
}
}
--
2.25.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] tracing: Remove redundant variable ret
2021-05-27 17:14 [PATCH] tracing: Remove redundant variable ret Hyeonggon Yoo
@ 2021-05-29 2:22 ` Steven Rostedt
2021-05-29 5:47 ` Hyeonggon Yoo
2021-05-29 6:00 ` [PATCH v2] tracing: Add WARN_ON_ONCE when returned value is negative Hyeonggon Yoo
2021-05-29 6:14 ` Hyeonggon Yoo
2 siblings, 1 reply; 6+ messages in thread
From: Steven Rostedt @ 2021-05-29 2:22 UTC (permalink / raw)
To: Hyeonggon Yoo; +Cc: Ingo Molnar, linux-kernel, Tom Zanussi
On Fri, 28 May 2021 02:14:49 +0900
Hyeonggon Yoo <42.hyeyoo@gmail.com> wrote:
> This variable saves return value of event_hist_trigger_func,
> but it's never read. So it's redundant.
>
> Signed-off-by: Hyeonggon Yoo <42.hyeyoo@gmail.com>
> ---
> kernel/trace/trace_events_hist.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
> index c1abd63f1d6c..414f2727d7a7 100644
> --- a/kernel/trace/trace_events_hist.c
> +++ b/kernel/trace/trace_events_hist.c
> @@ -5225,12 +5225,11 @@ static void unregister_field_var_hists(struct hist_trigger_data *hist_data)
> struct trace_event_file *file;
> unsigned int i;
> char *cmd;
> - int ret;
>
> for (i = 0; i < hist_data->n_field_var_hists; i++) {
> file = hist_data->field_var_hists[i]->hist_data->event_file;
> cmd = hist_data->field_var_hists[i]->cmd;
> - ret = event_hist_trigger_func(&trigger_hist_cmd, file,
> + event_hist_trigger_func(&trigger_hist_cmd, file,
> "!hist", "hist", cmd);
I wonder if instead we should add:
WARN_ON_ONCE(ret < 0);
-- Steve
> }
> }
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] tracing: Remove redundant variable ret
2021-05-29 2:22 ` Steven Rostedt
@ 2021-05-29 5:47 ` Hyeonggon Yoo
0 siblings, 0 replies; 6+ messages in thread
From: Hyeonggon Yoo @ 2021-05-29 5:47 UTC (permalink / raw)
To: Steven Rostedt; +Cc: Ingo Molnar, linux-kernel, Tom Zanussi
On Fri, May 28, 2021 at 10:22:57PM -0400, Steven Rostedt wrote:
> On Fri, 28 May 2021 02:14:49 +0900
> Hyeonggon Yoo <42.hyeyoo@gmail.com> wrote:
>
> > This variable saves return value of event_hist_trigger_func,
> > but it's never read. So it's redundant.
> >
> > Signed-off-by: Hyeonggon Yoo <42.hyeyoo@gmail.com>
> > ---
> > kernel/trace/trace_events_hist.c | 3 +--
> > 1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
> > index c1abd63f1d6c..414f2727d7a7 100644
> > --- a/kernel/trace/trace_events_hist.c
> > +++ b/kernel/trace/trace_events_hist.c
> > @@ -5225,12 +5225,11 @@ static void unregister_field_var_hists(struct hist_trigger_data *hist_data)
> > struct trace_event_file *file;
> > unsigned int i;
> > char *cmd;
> > - int ret;
> >
> > for (i = 0; i < hist_data->n_field_var_hists; i++) {
> > file = hist_data->field_var_hists[i]->hist_data->event_file;
> > cmd = hist_data->field_var_hists[i]->cmd;
> > - ret = event_hist_trigger_func(&trigger_hist_cmd, file,
> > + event_hist_trigger_func(&trigger_hist_cmd, file,
> > "!hist", "hist", cmd);
>
> I wonder if instead we should add:
>
> WARN_ON_ONCE(ret < 0);
>
Oh, Seems better to warn, not ignoring.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2] tracing: Add WARN_ON_ONCE when returned value is negative
2021-05-27 17:14 [PATCH] tracing: Remove redundant variable ret Hyeonggon Yoo
2021-05-29 2:22 ` Steven Rostedt
@ 2021-05-29 6:00 ` Hyeonggon Yoo
2021-05-29 6:14 ` Hyeonggon Yoo
2 siblings, 0 replies; 6+ messages in thread
From: Hyeonggon Yoo @ 2021-05-29 6:00 UTC (permalink / raw)
To: Steven Rostedt, Ingo Molnar; +Cc: linux-kernel
From af7124e2bd00f739010eb283c9ab07e07da52224 Mon Sep 17 00:00:00 2001
From: Hyeonggon Yoo <42.hyeyoo@gmail.com>
Date: Sat, 29 May 2021 14:51:05 +0900
Subject: [PATCH v2] tracing: Add WARN_ON_ONCE when returned value is negative
ret is assigned return value of event_hist_trigger_func, but the value
is unused. It is better to warn when returned value is negative,
rather than just ignoring it.
Signed-off-by: Hyeonggon Yoo <42.hyeyoo@gmail.com>
---
kernel/trace/trace_events_hist.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
index c1abd63f1d6c..d169946ea4e9 100644
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -5232,6 +5232,7 @@ static void unregister_field_var_hists(struct hist_trigger_data *hist_data)
cmd = hist_data->field_var_hists[i]->cmd;
ret = event_hist_trigger_func(&trigger_hist_cmd, file,
"!hist", "hist", cmd);
+ WARN_ON_ONCE(ret < 0);
}
}
--
2.25.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v2] tracing: Add WARN_ON_ONCE when returned value is negative
2021-05-27 17:14 [PATCH] tracing: Remove redundant variable ret Hyeonggon Yoo
2021-05-29 2:22 ` Steven Rostedt
2021-05-29 6:00 ` [PATCH v2] tracing: Add WARN_ON_ONCE when returned value is negative Hyeonggon Yoo
@ 2021-05-29 6:14 ` Hyeonggon Yoo
2021-05-29 6:14 ` Hyeonggon Yoo
2 siblings, 1 reply; 6+ messages in thread
From: Hyeonggon Yoo @ 2021-05-29 6:14 UTC (permalink / raw)
To: Steven Rostedt, Ingo Molnar; +Cc: linux-kernel
ret is assigned return value of event_hist_trigger_func, but the value
is unused. It is better to warn when returned value is negative,
rather than just ignoring it.
Signed-off-by: Hyeonggon Yoo <42.hyeyoo@gmail.com>
---
kernel/trace/trace_events_hist.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
index c1abd63f1d6c..d169946ea4e9 100644
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -5232,6 +5232,7 @@ static void unregister_field_var_hists(struct hist_trigger_data *hist_data)
cmd = hist_data->field_var_hists[i]->cmd;
ret = event_hist_trigger_func(&trigger_hist_cmd, file,
"!hist", "hist", cmd);
+ WARN_ON_ONCE(ret < 0);
}
}
--
2.25.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2] tracing: Add WARN_ON_ONCE when returned value is negative
2021-05-29 6:14 ` Hyeonggon Yoo
@ 2021-05-29 6:14 ` Hyeonggon Yoo
0 siblings, 0 replies; 6+ messages in thread
From: Hyeonggon Yoo @ 2021-05-29 6:14 UTC (permalink / raw)
To: Steven Rostedt, Ingo Molnar; +Cc: linux-kernel
Sorry, Sent it wrong format.
I think this is right format
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-05-29 6:17 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-27 17:14 [PATCH] tracing: Remove redundant variable ret Hyeonggon Yoo
2021-05-29 2:22 ` Steven Rostedt
2021-05-29 5:47 ` Hyeonggon Yoo
2021-05-29 6:00 ` [PATCH v2] tracing: Add WARN_ON_ONCE when returned value is negative Hyeonggon Yoo
2021-05-29 6:14 ` Hyeonggon Yoo
2021-05-29 6:14 ` Hyeonggon Yoo
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).