All of lore.kernel.org
 help / color / mirror / Atom feed
From: Yuntao Wang <ytcoode@gmail.com>
To: Lukas Bulwahn <lukas.bulwahn@gmail.com>
Cc: Steven Rostedt <rostedt@goodmis.org>,
	Ingo Molnar <mingo@redhat.com>,
	linux-kernel@vger.kernel.org, Yuntao Wang <ytcoode@gmail.com>
Subject: Re: [PATCH] tracing: Remove redundant assignment to variable ret
Date: Wed, 19 Jan 2022 04:17:37 +0000	[thread overview]
Message-ID: <20220119041737.1805441-1-ytcoode@gmail.com> (raw)
In-Reply-To: <20220117074117.3770-1-lukas.bulwahn@gmail.com>

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

  reply	other threads:[~2022-01-19  4:18 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-01-16 14:48 [PATCH] tracing: Remove redundant assignment to variable ret Yuntao Wang
2022-01-17  7:41 ` [PATCH] tracing: Remove redundant assignment to variable ret' Lukas Bulwahn
2022-01-19  4:17   ` Yuntao Wang [this message]
2022-01-19  7:15     ` [PATCH] tracing: Remove redundant assignment to variable ret Lukas Bulwahn
2022-01-19 14:57       ` Steven Rostedt
2022-01-20  6:59         ` [PATCH v2] tracing: Deal with error return code of the ftrace_process_locs() function Yuntao Wang
2022-05-24 17:24           ` Steven Rostedt

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220119041737.1805441-1-ytcoode@gmail.com \
    --to=ytcoode@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lukas.bulwahn@gmail.com \
    --cc=mingo@redhat.com \
    --cc=rostedt@goodmis.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.