lkml.org 
[lkml]   [2022]   [Jul]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH] tracing/histograms: Simplify create_hist_fields()
Date
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>

\
 
 \ /
  Last update: 2022-07-13 12:35    [W:0.086 / U:0.020 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site