* [PATCH] Silence an uninitialized variable warning @ 2019-11-26 12:19 ` Dan Carpenter 0 siblings, 0 replies; 10+ messages in thread From: Dan Carpenter @ 2019-11-26 12:19 UTC (permalink / raw) To: Steven Rostedt; +Cc: Ingo Molnar, linux-kernel, kernel-janitors Smatch complains that "ret" could be uninitialized if we don't enter the loop. I don't know if that's possible, but it's nicer to return a literal zero instead. Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> --- kernel/trace/trace_syscalls.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kernel/trace/trace_syscalls.c b/kernel/trace/trace_syscalls.c index 73140d80dd46..63528f458826 100644 --- a/kernel/trace/trace_syscalls.c +++ b/kernel/trace/trace_syscalls.c @@ -286,7 +286,7 @@ static int __init syscall_enter_define_fields(struct trace_event_call *call) offset += sizeof(unsigned long); } - return ret; + return 0; } static void ftrace_syscall_enter(void *data, struct pt_regs *regs, long id) -- 2.11.0 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH] Silence an uninitialized variable warning @ 2019-11-26 12:19 ` Dan Carpenter 0 siblings, 0 replies; 10+ messages in thread From: Dan Carpenter @ 2019-11-26 12:19 UTC (permalink / raw) To: Steven Rostedt; +Cc: Ingo Molnar, linux-kernel, kernel-janitors Smatch complains that "ret" could be uninitialized if we don't enter the loop. I don't know if that's possible, but it's nicer to return a literal zero instead. Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> --- kernel/trace/trace_syscalls.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kernel/trace/trace_syscalls.c b/kernel/trace/trace_syscalls.c index 73140d80dd46..63528f458826 100644 --- a/kernel/trace/trace_syscalls.c +++ b/kernel/trace/trace_syscalls.c @@ -286,7 +286,7 @@ static int __init syscall_enter_define_fields(struct trace_event_call *call) offset += sizeof(unsigned long); } - return ret; + return 0; } static void ftrace_syscall_enter(void *data, struct pt_regs *regs, long id) -- 2.11.0 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] Silence an uninitialized variable warning 2019-11-26 12:19 ` Dan Carpenter @ 2019-12-04 14:26 ` Steven Rostedt -1 siblings, 0 replies; 10+ messages in thread From: Steven Rostedt @ 2019-12-04 14:26 UTC (permalink / raw) To: Dan Carpenter; +Cc: Ingo Molnar, linux-kernel, kernel-janitors On Tue, 26 Nov 2019 15:19:34 +0300 Dan Carpenter <dan.carpenter@oracle.com> wrote: > Smatch complains that "ret" could be uninitialized if we don't enter the > loop. I don't know if that's possible, but it's nicer to return a > literal zero instead. > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> > --- > kernel/trace/trace_syscalls.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/kernel/trace/trace_syscalls.c b/kernel/trace/trace_syscalls.c > index 73140d80dd46..63528f458826 100644 > --- a/kernel/trace/trace_syscalls.c > +++ b/kernel/trace/trace_syscalls.c > @@ -286,7 +286,7 @@ static int __init syscall_enter_define_fields(struct trace_event_call *call) > offset += sizeof(unsigned long); > } > > - return ret; > + return 0; > } > > static void ftrace_syscall_enter(void *data, struct pt_regs *regs, long id) The current code has this: static int __init syscall_enter_define_fields(struct trace_event_call *call) { struct syscall_trace_enter trace; struct syscall_metadata *meta = call->data; int ret; int i; int offset = offsetof(typeof(trace), args); ret = trace_define_field(call, SYSCALL_FIELD(int, nr, __syscall_nr), FILTER_OTHER); if (ret) return ret; for (i = 0; i < meta->nb_args; i++) { ret = trace_define_field(call, meta->types[i], meta->args[i], offset, sizeof(unsigned long), 0, FILTER_OTHER); offset += sizeof(unsigned long); } return ret; } How can ret possibly be uninitialized? -- Steve ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Silence an uninitialized variable warning @ 2019-12-04 14:26 ` Steven Rostedt 0 siblings, 0 replies; 10+ messages in thread From: Steven Rostedt @ 2019-12-04 14:26 UTC (permalink / raw) To: Dan Carpenter; +Cc: Ingo Molnar, linux-kernel, kernel-janitors On Tue, 26 Nov 2019 15:19:34 +0300 Dan Carpenter <dan.carpenter@oracle.com> wrote: > Smatch complains that "ret" could be uninitialized if we don't enter the > loop. I don't know if that's possible, but it's nicer to return a > literal zero instead. > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> > --- > kernel/trace/trace_syscalls.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/kernel/trace/trace_syscalls.c b/kernel/trace/trace_syscalls.c > index 73140d80dd46..63528f458826 100644 > --- a/kernel/trace/trace_syscalls.c > +++ b/kernel/trace/trace_syscalls.c > @@ -286,7 +286,7 @@ static int __init syscall_enter_define_fields(struct trace_event_call *call) > offset += sizeof(unsigned long); > } > > - return ret; > + return 0; > } > > static void ftrace_syscall_enter(void *data, struct pt_regs *regs, long id) The current code has this: static int __init syscall_enter_define_fields(struct trace_event_call *call) { struct syscall_trace_enter trace; struct syscall_metadata *meta = call->data; int ret; int i; int offset = offsetof(typeof(trace), args); ret = trace_define_field(call, SYSCALL_FIELD(int, nr, __syscall_nr), FILTER_OTHER); if (ret) return ret; for (i = 0; i < meta->nb_args; i++) { ret = trace_define_field(call, meta->types[i], meta->args[i], offset, sizeof(unsigned long), 0, FILTER_OTHER); offset += sizeof(unsigned long); } return ret; } How can ret possibly be uninitialized? -- Steve ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Silence an uninitialized variable warning 2019-12-04 14:26 ` Steven Rostedt @ 2019-12-04 18:42 ` Dan Carpenter -1 siblings, 0 replies; 10+ messages in thread From: Dan Carpenter @ 2019-12-04 18:42 UTC (permalink / raw) To: Steven Rostedt; +Cc: Ingo Molnar, linux-kernel, kernel-janitors, Peter Zijlstra On Wed, Dec 04, 2019 at 09:26:40AM -0500, Steven Rostedt wrote: > On Tue, 26 Nov 2019 15:19:34 +0300 > Dan Carpenter <dan.carpenter@oracle.com> wrote: > > > Smatch complains that "ret" could be uninitialized if we don't enter the > > loop. I don't know if that's possible, but it's nicer to return a > > literal zero instead. > > > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> > > --- > > kernel/trace/trace_syscalls.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/kernel/trace/trace_syscalls.c b/kernel/trace/trace_syscalls.c > > index 73140d80dd46..63528f458826 100644 > > --- a/kernel/trace/trace_syscalls.c > > +++ b/kernel/trace/trace_syscalls.c > > @@ -286,7 +286,7 @@ static int __init syscall_enter_define_fields(struct trace_event_call *call) > > offset += sizeof(unsigned long); > > } > > > > - return ret; > > + return 0; > > } > > > > static void ftrace_syscall_enter(void *data, struct pt_regs *regs, long id) > > The current code has this: > > static int __init syscall_enter_define_fields(struct trace_event_call *call) > { > struct syscall_trace_enter trace; > struct syscall_metadata *meta = call->data; > int ret; > int i; > int offset = offsetof(typeof(trace), args); > > ret = trace_define_field(call, SYSCALL_FIELD(int, nr, __syscall_nr), > FILTER_OTHER); In linux-next this ret = trace_define_field() assignment is removed. That was commit 60fdad00827c ("ftrace: Rework event_create_dir()"). > if (ret) > return ret; > > for (i = 0; i < meta->nb_args; i++) { > ret = trace_define_field(call, meta->types[i], > meta->args[i], offset, > sizeof(unsigned long), 0, > FILTER_OTHER); > offset += sizeof(unsigned long); > } > > return ret; > } > > > How can ret possibly be uninitialized? I should have written this commit more carefully and verified whether meta->nb_args can actually be zero instead of just assuming it was a false positive... regards, dan carpenter ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Silence an uninitialized variable warning @ 2019-12-04 18:42 ` Dan Carpenter 0 siblings, 0 replies; 10+ messages in thread From: Dan Carpenter @ 2019-12-04 18:42 UTC (permalink / raw) To: Steven Rostedt; +Cc: Ingo Molnar, linux-kernel, kernel-janitors, Peter Zijlstra On Wed, Dec 04, 2019 at 09:26:40AM -0500, Steven Rostedt wrote: > On Tue, 26 Nov 2019 15:19:34 +0300 > Dan Carpenter <dan.carpenter@oracle.com> wrote: > > > Smatch complains that "ret" could be uninitialized if we don't enter the > > loop. I don't know if that's possible, but it's nicer to return a > > literal zero instead. > > > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> > > --- > > kernel/trace/trace_syscalls.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/kernel/trace/trace_syscalls.c b/kernel/trace/trace_syscalls.c > > index 73140d80dd46..63528f458826 100644 > > --- a/kernel/trace/trace_syscalls.c > > +++ b/kernel/trace/trace_syscalls.c > > @@ -286,7 +286,7 @@ static int __init syscall_enter_define_fields(struct trace_event_call *call) > > offset += sizeof(unsigned long); > > } > > > > - return ret; > > + return 0; > > } > > > > static void ftrace_syscall_enter(void *data, struct pt_regs *regs, long id) > > The current code has this: > > static int __init syscall_enter_define_fields(struct trace_event_call *call) > { > struct syscall_trace_enter trace; > struct syscall_metadata *meta = call->data; > int ret; > int i; > int offset = offsetof(typeof(trace), args); > > ret = trace_define_field(call, SYSCALL_FIELD(int, nr, __syscall_nr), > FILTER_OTHER); In linux-next this ret = trace_define_field() assignment is removed. That was commit 60fdad00827c ("ftrace: Rework event_create_dir()"). > if (ret) > return ret; > > for (i = 0; i < meta->nb_args; i++) { > ret = trace_define_field(call, meta->types[i], > meta->args[i], offset, > sizeof(unsigned long), 0, > FILTER_OTHER); > offset += sizeof(unsigned long); > } > > return ret; > } > > > How can ret possibly be uninitialized? I should have written this commit more carefully and verified whether meta->nb_args can actually be zero instead of just assuming it was a false positive... regards, dan carpenter ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Silence an uninitialized variable warning 2019-12-04 18:42 ` Dan Carpenter @ 2019-12-05 9:32 ` Peter Zijlstra -1 siblings, 0 replies; 10+ messages in thread From: Peter Zijlstra @ 2019-12-05 9:32 UTC (permalink / raw) To: Dan Carpenter; +Cc: Steven Rostedt, Ingo Molnar, linux-kernel, kernel-janitors On Wed, Dec 04, 2019 at 09:42:47PM +0300, Dan Carpenter wrote: > > The current code has this: > > > > static int __init syscall_enter_define_fields(struct trace_event_call *call) > > { > > struct syscall_trace_enter trace; > > struct syscall_metadata *meta = call->data; > > int ret; > > int i; > > int offset = offsetof(typeof(trace), args); > > > > ret = trace_define_field(call, SYSCALL_FIELD(int, nr, __syscall_nr), > > FILTER_OTHER); > > In linux-next this ret = trace_define_field() assignment is removed. > That was commit 60fdad00827c ("ftrace: Rework event_create_dir()"). Yep, mea culpa. > > if (ret) > > return ret; > > > > for (i = 0; i < meta->nb_args; i++) { > > ret = trace_define_field(call, meta->types[i], > > meta->args[i], offset, > > sizeof(unsigned long), 0, > > FILTER_OTHER); > > offset += sizeof(unsigned long); > > } > > > > return ret; > > } > > > > > > How can ret possibly be uninitialized? > > I should have written this commit more carefully and verified whether > meta->nb_args can actually be zero instead of just assuming it was a > false positive... Right, I'm thinking this is in fact possible. We have syscalls without arguments (sys_sched_yield for exmaple). ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Silence an uninitialized variable warning @ 2019-12-05 9:32 ` Peter Zijlstra 0 siblings, 0 replies; 10+ messages in thread From: Peter Zijlstra @ 2019-12-05 9:32 UTC (permalink / raw) To: Dan Carpenter; +Cc: Steven Rostedt, Ingo Molnar, linux-kernel, kernel-janitors On Wed, Dec 04, 2019 at 09:42:47PM +0300, Dan Carpenter wrote: > > The current code has this: > > > > static int __init syscall_enter_define_fields(struct trace_event_call *call) > > { > > struct syscall_trace_enter trace; > > struct syscall_metadata *meta = call->data; > > int ret; > > int i; > > int offset = offsetof(typeof(trace), args); > > > > ret = trace_define_field(call, SYSCALL_FIELD(int, nr, __syscall_nr), > > FILTER_OTHER); > > In linux-next this ret = trace_define_field() assignment is removed. > That was commit 60fdad00827c ("ftrace: Rework event_create_dir()"). Yep, mea culpa. > > if (ret) > > return ret; > > > > for (i = 0; i < meta->nb_args; i++) { > > ret = trace_define_field(call, meta->types[i], > > meta->args[i], offset, > > sizeof(unsigned long), 0, > > FILTER_OTHER); > > offset += sizeof(unsigned long); > > } > > > > return ret; > > } > > > > > > How can ret possibly be uninitialized? > > I should have written this commit more carefully and verified whether > meta->nb_args can actually be zero instead of just assuming it was a > false positive... Right, I'm thinking this is in fact possible. We have syscalls without arguments (sys_sched_yield for exmaple). ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Silence an uninitialized variable warning 2019-12-05 9:32 ` Peter Zijlstra @ 2019-12-05 10:02 ` Dan Carpenter -1 siblings, 0 replies; 10+ messages in thread From: Dan Carpenter @ 2019-12-05 10:02 UTC (permalink / raw) To: Peter Zijlstra; +Cc: Steven Rostedt, Ingo Molnar, linux-kernel, kernel-janitors On Thu, Dec 05, 2019 at 10:32:29AM +0100, Peter Zijlstra wrote: > On Wed, Dec 04, 2019 at 09:42:47PM +0300, Dan Carpenter wrote: > > > > The current code has this: > > > > > > static int __init syscall_enter_define_fields(struct trace_event_call *call) > > > { > > > struct syscall_trace_enter trace; > > > struct syscall_metadata *meta = call->data; > > > int ret; > > > int i; > > > int offset = offsetof(typeof(trace), args); > > > > > > ret = trace_define_field(call, SYSCALL_FIELD(int, nr, __syscall_nr), > > > FILTER_OTHER); > > > > In linux-next this ret = trace_define_field() assignment is removed. > > That was commit 60fdad00827c ("ftrace: Rework event_create_dir()"). > > Yep, mea culpa. > > > > if (ret) > > > return ret; > > > > > > for (i = 0; i < meta->nb_args; i++) { > > > ret = trace_define_field(call, meta->types[i], > > > meta->args[i], offset, > > > sizeof(unsigned long), 0, > > > FILTER_OTHER); > > > offset += sizeof(unsigned long); > > > } > > > > > > return ret; > > > } > > > > > > > > > How can ret possibly be uninitialized? > > > > I should have written this commit more carefully and verified whether > > meta->nb_args can actually be zero instead of just assuming it was a > > false positive... > > Right, I'm thinking this is in fact possible. We have syscalls without > arguments (sys_sched_yield for exmaple). Well, it would have triggered a run time bug because of that thing with GCC where it sometimes initializes variables to zero. Let me resend properly with a Fixes tag. regards, dan carpenter ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Silence an uninitialized variable warning @ 2019-12-05 10:02 ` Dan Carpenter 0 siblings, 0 replies; 10+ messages in thread From: Dan Carpenter @ 2019-12-05 10:02 UTC (permalink / raw) To: Peter Zijlstra; +Cc: Steven Rostedt, Ingo Molnar, linux-kernel, kernel-janitors On Thu, Dec 05, 2019 at 10:32:29AM +0100, Peter Zijlstra wrote: > On Wed, Dec 04, 2019 at 09:42:47PM +0300, Dan Carpenter wrote: > > > > The current code has this: > > > > > > static int __init syscall_enter_define_fields(struct trace_event_call *call) > > > { > > > struct syscall_trace_enter trace; > > > struct syscall_metadata *meta = call->data; > > > int ret; > > > int i; > > > int offset = offsetof(typeof(trace), args); > > > > > > ret = trace_define_field(call, SYSCALL_FIELD(int, nr, __syscall_nr), > > > FILTER_OTHER); > > > > In linux-next this ret = trace_define_field() assignment is removed. > > That was commit 60fdad00827c ("ftrace: Rework event_create_dir()"). > > Yep, mea culpa. > > > > if (ret) > > > return ret; > > > > > > for (i = 0; i < meta->nb_args; i++) { > > > ret = trace_define_field(call, meta->types[i], > > > meta->args[i], offset, > > > sizeof(unsigned long), 0, > > > FILTER_OTHER); > > > offset += sizeof(unsigned long); > > > } > > > > > > return ret; > > > } > > > > > > > > > How can ret possibly be uninitialized? > > > > I should have written this commit more carefully and verified whether > > meta->nb_args can actually be zero instead of just assuming it was a > > false positive... > > Right, I'm thinking this is in fact possible. We have syscalls without > arguments (sys_sched_yield for exmaple). Well, it would have triggered a run time bug because of that thing with GCC where it sometimes initializes variables to zero. Let me resend properly with a Fixes tag. regards, dan carpenter ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2019-12-05 10:03 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2019-11-26 12:19 [PATCH] Silence an uninitialized variable warning Dan Carpenter 2019-11-26 12:19 ` Dan Carpenter 2019-12-04 14:26 ` Steven Rostedt 2019-12-04 14:26 ` Steven Rostedt 2019-12-04 18:42 ` Dan Carpenter 2019-12-04 18:42 ` Dan Carpenter 2019-12-05 9:32 ` Peter Zijlstra 2019-12-05 9:32 ` Peter Zijlstra 2019-12-05 10:02 ` Dan Carpenter 2019-12-05 10:02 ` 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.