2021-05-27 23:24:35

by Hyeonggon Yoo

Subject: [PATCH] tracing: Remove redundant variable ret

This variable saves return value of event_hist_trigger_func,
but it's never read. So it's redundant.

Signed-off-by: Hyeonggon Yoo <[email protected]>
---
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

2021-05-29 02:23:48

by Steven Rostedt

Subject: Re: [PATCH] tracing: Remove redundant variable ret

On Fri, 28 May 2021 02:14:49 +0900
Hyeonggon Yoo <[email protected]> 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 <[email protected]>
> ---
> 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

> }
> }

2021-05-29 05:49:45

by Hyeonggon Yoo

Subject: Re: [PATCH] tracing: Remove redundant variable ret

On Fri, May 28, 2021 at 10:22:57PM -0400, Steven Rostedt wrote:
> On Fri, 28 May 2021 02:14:49 +0900
> Hyeonggon Yoo <[email protected]> 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 <[email protected]>
> > ---
> > 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.

2021-05-29 06:08:39

by Hyeonggon Yoo

Subject: [PATCH v2] tracing: Add WARN_ON_ONCE when returned value is negative

From af7124e2bd00f739010eb283c9ab07e07da52224 Mon Sep 17 00:00:00 2001
From: Hyeonggon Yoo <[email protected]>
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 <[email protected]>
---
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

2021-05-29 06:15:17

by Hyeonggon Yoo

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 <[email protected]>
---
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

2021-05-29 06:15:51

by Hyeonggon Yoo

Subject: Re: [PATCH v2] tracing: Add WARN_ON_ONCE when returned value is negative

Sorry, Sent it wrong format.
I think this is right format