Re: [PATCH] perf stat: fix cvs output format

[Cong Wang]

On Tue, Mar 6, 2018 at 9:53 AM, Andi Kleen <a...@linux.intel.com> wrote:
>> Here is the output from your own commit:
>>
>>       423470,,stalled-cycles-frontend,509102,100.00,65.69,frontend cycles 
>> idle
>>       <not supported>,,stalled-cycles-backend,0,100.00,,,,
>>
>> so line 1 has 7 fields, line 2 has 9 fields, and this is expected?
>
> If you had metrics on line 1 it would be correct.
>
> So you just shifted it to break that case.
>
> If you always want to have the same number of fields
> you need to add two empty fields to the normal output
> when there are no metrics.

The number of separators is the only way to learn the number
of fields, therefore it must be a fixed number.

Yeah, it could be the other way that supported ones have less
separators than it should. If we look at print_metric_csv() alone,
it should produce a same number of separators for all cases,
otherwise hard to count.

So I believe we need an additional patch, like the one attached,
to make it complete? Note, I only spot the cgroup field here.

diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index 54a4c152edb3..6896e739ae4e 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -1044,8 +1044,7 @@ static void nsec_printout(int id, int nr, struct 
perf_evsel *evsel, double avg)
 
        fprintf(output, fmt_n, name);
 
-       if (evsel->cgrp)
-               fprintf(output, "%s%s", csv_sep, evsel->cgrp->name);
+       fprintf(output, "%s%s", csv_sep, evsel->cgrp ? evsel->cgrp->name : "");
 }
 
 static int first_shadow_cpu(struct perf_evsel *evsel, int id)
@@ -1092,12 +1091,13 @@ static void abs_printout(int id, int nr, struct 
perf_evsel *evsel, double avg)
        if (evsel->unit)
                fprintf(output, "%-*s%s",
                        csv_output ? 0 : unit_width,
-                       evsel->unit, csv_sep);
+                       evsel->unit ? evsel->unit : "", csv_sep);
+       else
+               fprintf(output, "%s", csv_sep);
 
        fprintf(output, "%-*s", csv_output ? 0 : 25, perf_evsel__name(evsel));
 
-       if (evsel->cgrp)
-               fprintf(output, "%s%s", csv_sep, evsel->cgrp->name);
+       fprintf(output, "%s%s", csv_sep, evsel->cgrp ? evsel->cgrp->name : "");
 }
 
 static void printout(int id, int nr, struct perf_evsel *counter, double uval,
@@ -1163,9 +1163,8 @@ static void printout(int id, int nr, struct perf_evsel 
*counter, double uval,
                        csv_output ? 0 : -25,
                        perf_evsel__name(counter));
 
-               if (counter->cgrp)
-                       fprintf(stat_config.output, "%s%s",
-                               csv_sep, counter->cgrp->name);
+               fprintf(stat_config.output, "%s%s", csv_sep,
+                       counter->cgrp ? counter->cgrp->name : "");
 
                if (!csv_output)
                        pm(&os, NULL, NULL, "", 0);