linux-kernel - Re: [PATCH V2 06/12] perf mem: Clean up output format

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20201208102943.GB4135722@krava>
Date:   Tue, 8 Dec 2020 11:29:43 +0100
From:   Jiri Olsa <jolsa@...hat.com>
To:     "Liang, Kan" <kan.liang@...ux.intel.com>
Cc:     acme@...nel.org, mingo@...nel.org, linux-kernel@...r.kernel.org,
        namhyung@...nel.org, eranian@...gle.com, ak@...ux.intel.com,
        mark.rutland@....com, will@...nel.org, mpe@...erman.id.au
Subject: Re: [PATCH V2 06/12] perf mem: Clean up output format

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, kan.liang@...ux.intel.com 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