On Mon, Dec 07, 2020 at 03:19:06PM -0500, Liang, Kan wrote:
> 
> 
> On 12/4/2020 6:27 PM, Jiri Olsa wrote:
> > On Mon, Nov 30, 2020 at 09:27:57AM -0800, [email protected] wrote:
> > 
> > SNIP
> > 
> > > @@ -172,7 +172,7 @@ dump_raw_samples(struct perf_tool *tool,
> > >   {
> > >           struct perf_mem *mem = container_of(tool, struct perf_mem, 
> > > tool);
> > >           struct addr_location al;
> > > - const char *fmt;
> > > + const char *fmt, *field_sep;
> > >           if (machine__resolve(machine, &al, sample) < 0) {
> > >                   fprintf(stderr, "problem processing %d event, skipping 
> > > it.\n",
> > > @@ -186,60 +186,41 @@ dump_raw_samples(struct perf_tool *tool,
> > >           if (al.map != NULL)
> > >                   al.map->dso->hit = 1;
> > > - if (mem->phys_addr) {
> > > -         if (symbol_conf.field_sep) {
> > > -                 fmt = "%d%s%d%s0x%"PRIx64"%s0x%"PRIx64"%s0x%016"PRIx64
> > > -                       "%s%"PRIu64"%s0x%"PRIx64"%s%s:%s\n";
> > > -         } else {
> > > -                 fmt = "%5d%s%5d%s0x%016"PRIx64"%s0x016%"PRIx64
> > > -                       "%s0x%016"PRIx64"%s%5"PRIu64"%s0x%06"PRIx64
> > > -                       "%s%s:%s\n";
> > > -                 symbol_conf.field_sep = " ";
> > > -         }
> > > -
> > > -         printf(fmt,
> > > -                 sample->pid,
> > > -                 symbol_conf.field_sep,
> > > -                 sample->tid,
> > > -                 symbol_conf.field_sep,
> > > -                 sample->ip,
> > > -                 symbol_conf.field_sep,
> > > -                 sample->addr,
> > > -                 symbol_conf.field_sep,
> > > -                 sample->phys_addr,
> > > -                 symbol_conf.field_sep,
> > > -                 sample->weight,
> > > -                 symbol_conf.field_sep,
> > > -                 sample->data_src,
> > > -                 symbol_conf.field_sep,
> > > -                 al.map ? (al.map->dso ? al.map->dso->long_name : "???") 
> > > : "???",
> > > -                 al.sym ? al.sym->name : "???");
> > > + field_sep = symbol_conf.field_sep;
> > 
> > hum, what's the point of having field_sep?
> 
> 
> To keep the fmt consistent.
> 
> The patch divides the "printf(fmt,..." into two part. In the first half
> part, the symbol_conf.field_sep may be modified to " ";
> In the second half part, we should not use the modified
> symbol_conf.field_sep for the below check. The field_sep keeps the original
> value and should be used.

ok, I missed it's being moified.. thanks

jirka

Reply via email to