Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Zheng Yejian <zhengyejian1@huawei.com>
To:     <rostedt@goodmis.org>
CC:     <linux-kernel@vger.kernel.org>, <mingo@redhat.com>,
        <stable@vger.kernel.org>, <tom.zanussi@linux.intel.com>,
        <trix@redhat.com>, <zhangjinhao2@huawei.com>,
        <zhengyejian1@huawei.com>
Subject: Re: [PATCH] tracing/histograms: Simplify create_hist_fields()
Date:   Wed, 13 Jul 2022 18:32:35 +0800
Message-ID: <20220713103235.129358-1-zhengyejian1@huawei.com>
In-Reply-To: <20220712134956.4acb90a3@gandalf.local.home>
References: <20220712134956.4acb90a3@gandalf.local.home>
MIME-Version: 1.0
Content-Transfer-Encoding: 7BIT
Content-Type:   text/plain; charset=US-ASCII
Precedence: bulk

On Tue, 12 Jul 2022 13:49:56 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:
> On Thu, 30 Jun 2022 09:31:52 +0800
> Zheng Yejian <zhengyejian1@huawei.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@huawei.com>