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] [day] [month] [year] [list]

Date:   Wed, 13 Jul 2022 18:32:35 +0800
From:   Zheng Yejian <zhengyejian1@...wei.com>
To:     <rostedt@...dmis.org>
CC:     <linux-kernel@...r.kernel.org>, <mingo@...hat.com>,
        <stable@...r.kernel.org>, <tom.zanussi@...ux.intel.com>,
        <trix@...hat.com>, <zhangjinhao2@...wei.com>,
        <zhengyejian1@...wei.com>
Subject: Re: [PATCH] tracing/histograms: Simplify create_hist_fields()

On Tue, 12 Jul 2022 13:49:56 -0400
Steven Rostedt <rostedt@...dmis.org> wrote:
> 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 )

I learn it now and share it with my colleagues in the neighborhood.
Thanks for your patience :)

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