On Sun, Nov 24, 2013 at 11:50:23AM +0530, Ramkumar Ramachandra wrote: > Generalize the function so that we can accommodate all three comparison > methods: delta, ratio, and wdiff. > > Cc: Jiri Olsa <jo...@redhat.com> > Cc: Namhyung Kim <namhy...@kernel.org> > Cc: Arnaldo Carvalho de Melo <a...@redhat.com> > Signed-off-by: Ramkumar Ramachandra <artag...@gmail.com> > --- > tools/perf/builtin-diff.c | 47 > +++++++++++++++++++++++++++++++++++++---------- > 1 file changed, 37 insertions(+), 10 deletions(-) > > diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c > index 84d5f2d..a54736c 100644 > --- a/tools/perf/builtin-diff.c > +++ b/tools/perf/builtin-diff.c > @@ -769,33 +769,60 @@ static int hpp__entry_baseline(struct hist_entry *he, > char *buf, size_t size) > return ret; > } > > -static int hpp__color_delta(struct perf_hpp_fmt *fmt, > - struct perf_hpp *hpp, struct hist_entry *he) > +static int __hpp__color_compare(struct perf_hpp_fmt *fmt, > + struct perf_hpp *hpp, struct hist_entry *he, > + int comparison_method) > { > struct diff_hpp_fmt *dfmt = > container_of(fmt, struct diff_hpp_fmt, fmt); > struct hist_entry *pair = get_pair_fmt(he, dfmt); > - double percent; > + double delta; > char pfmt[20] = " "; > > if (!pair) > goto dummy_print; > if (pair->diff.computed) > - percent = pair->diff.period_ratio_delta; > + switch (comparison_method) { > + case COMPUTE_DELTA: > + delta = pair->diff.period_ratio_delta; > + break; > + default: > + BUG_ON(1); > + } > else > - percent = compute_delta(he, pair); > + switch (comparison_method) { > + case COMPUTE_DELTA: > + delta = compute_delta(he, pair); > + break; > + default: > + BUG_ON(1); > + } > > - if (!he->dummy && fabs(percent) >= 0.01) { > - scnprintf(pfmt, 20, "%%%+d.2f%%%%", dfmt->header_width - 1); > - return color_snprintf(hpp->buf, hpp->size, > - percent > 0 ? PERF_COLOR_GREEN : PERF_COLOR_RED, > - pfmt, percent); > + if (!he->dummy) {
isn't this check superfluous because of the above (!pair) check? jirka -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/