linux-kernel - Re: [PATCH] tracing/histograms: Simplify create_hist

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20220712134956.4acb90a3@gandalf.local.home>
Date:   Tue, 12 Jul 2022 13:49:56 -0400
From:   Steven Rostedt <rostedt@...dmis.org>
To:     Zheng Yejian <zhengyejian1@...wei.com>
Cc:     <mingo@...hat.com>, <linux-kernel@...r.kernel.org>,
        <tom.zanussi@...ux.intel.com>, <trix@...hat.com>,
        <stable@...r.kernel.org>, <zhangjinhao2@...wei.com>
Subject: Re: [PATCH] tracing/histograms: Simplify create_hist_fields()

On Thu, 30 Jun 2022 09:31:52 +0800
Zheng Yejian <zhengyejian1@...wei.com> wrote:

> When I look into implements of create_hist_fields(), I think there can be
> following two simplifications:
>   1. If something wrong happened in parse_var_defs(), free_var_defs() would
>      have been called in it, so no need goto free again after calling it;
>   2. After calling create_key_fields(), regardless of the value of 'ret', it
>      then always runs into 'out: ', so the judge of 'ret' is redundant.
> 
> No functional changes.

I applied this but removed the "No functional changes" because it is a
functional change. The end result may be the same, but the flow is
different, and that means it changed functionally.

The only time "No functional changes" should be stated is if you move code
around or change #ifdefs to perform the same action. IOW, if the assembly
produced by the compiler is the same before and after your change, you can
say "No functional changes", otherwise don't ever say that.

This is important, because if a bisect lands on this, people may think the
bisect is incorrect, when in reality it could be the cause of the bug (I
just had this happen to me with another commit that had "No functional
changes" :-p )

-- Steve


> 
> Signed-off-by: Zheng Yejian <zhengyejian1@...wei.com>