Messages in this thread | | | From | Yuntao Wang <> | Subject | Re: [PATCH] tracing: Remove redundant assignment to variable ret | Date | Wed, 19 Jan 2022 04:17:37 +0000 |
| |
On Mon, Jan 17, 2022 at 3:47 PM Lukas Bulwahn <lukas.bulwahn@gmail.com> wrote: > Dear Yuntao, > > > when you consider removing dead-store assignments guided by some static > analyzer, you need to check if the code you are looking at is actually > missing an error-handling branch. > > In this case, ftrace_process_locs() may return -ENOMEM, and the caller > needs to appropriately deal with this error return code. Your patch > does not change the code at all, i.e., the compiled object code is the > same as after the patch as before. > > Think about how to deal appropriately with the -ENOMEM return in this > caller and submit a patch that implements the right error-handling > branch or argue in your commit message why that is not needed at all. > > If you do not understand or cannot check such basic code properties for > dead-store assignments, it might be better to work on some other aspect > and area of the kernel repository. E.g., the kernel documentation build > also has a few warnings that deserve patches to be fixed. > > > Best regards, > > Lukas
Dear Lukas,
Thanks for your reply.
Actually, I had read the source code carefully and noticed the possible error return code -ENOMEM of the ftrace_process_locs() function.
At first I was going to implement an error-handling branch as you said, but after digging into more details, I discovered:
- The ftrace_init() function did not handle the error return code of the ftrace_process_locs() function since the first version. - The ftrace_module_init() function did not handle it either.
To keep consistent with the existing code, I just removed the assignment in that patch.
Maybe we should deal with the error return code more appropriately, at least print some warnings?
Best regards,
Yuntao
| |