Re: [PATCH] tracing/histograms: Simplify create_hist_fields()

[Zheng Yejian]

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