On Mon, Feb 25, 2019 at 09:42:42PM +0800, Jin Yao wrote:
SNIP
> COMPARISON
> ----------
> The comparison is governed by the baseline file. The baseline perf.data
> diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
> index 751e197..ddc41e7 100644
> --- a/tools/perf/builtin-diff.c
> +++ b/tools/perf/builtin-diff.c
> @@ -19,12 +19,21 @@
> #include "util/util.h"
> #include "util/data.h"
> #include "util/config.h"
> +#include "util/time-utils.h"
>
> #include <errno.h>
> #include <inttypes.h>
> #include <stdlib.h>
> #include <math.h>
>
> +struct perf_diff {
> + struct perf_tool tool;
> + const char *time_str;
> + struct perf_time_interval *ptime_range;
> + int range_size;
> + int range_num;
please align the members
> static int __cmd_diff(void)
> {
> struct data__file *d;
> int ret = -EINVAL, i;
> + char *abstime_ostr = NULL, *abstime_tmp = NULL;
> + bool abstime = false;
> +
> + if (pdiff.time_str && strchr(pdiff.time_str, ':')) {
> + abstime = true;
> + pdiff.ptime_range = perf_time__range_alloc(NULL,
> + &pdiff.range_size);
> + } else {
> + pdiff.ptime_range = perf_time__range_alloc(pdiff.time_str,
> + &pdiff.range_size);
> + }
> +
> + if (!pdiff.ptime_range)
> + return -ENOMEM;
> +
> + if (abstime) {
> + abstime_ostr = strdup(pdiff.time_str);
> + if (!abstime_ostr) {
> + zfree(&pdiff.ptime_range);
> + return -ENOMEM;
> + }
> + abstime_tmp = abstime_ostr;
> + }
could you please put this in separate function?
>
> data__for_each_file(i, d) {
> - d->session = perf_session__new(&d->data, false, &tool);
> + d->session = perf_session__new(&d->data, false, &pdiff.tool);
> if (!d->session) {
> pr_err("Failed to open %s\n", d->data.file.path);
> ret = -1;
> goto out_delete;
> }
>
> + if (abstime) {
> + ret = parse_absolute_time(d, &abstime_tmp);
> + if (ret < 0)
> + goto out_delete;
> + } else if (pdiff.time_str) {
> + ret = parse_percent_time(d);
> + if (ret < 0)
> + goto out_delete;
> + } else {
> + pdiff.range_num = 1;
> + }
> +
and this as well
thanks,
jirka
