[PATCH 0/4] perf tools: runtime fixes for Android

[PATCH 0/4] perf tools: runtime fixes for Android

[Irina Tirdea]

This is a set of patches with runtime fixes for Android.

Any comments and suggestions are wellcome.

Thanks,
Irina

Irina Tirdea (4):
  perf tools: remove sscanf extension %as
  perf stat: add compile-time option to disable --big-num
  perf archive: remove -f from the rm command
  perf archive: make f the last parameter for tar

 tools/perf/Makefile                 |    8 ++++++++
 tools/perf/builtin-stat.c           |    8 ++++++++
 tools/perf/config/feature-tests.mak |   12 ++++++++++++
 tools/perf/perf-archive.sh          |    6 +++---
 tools/perf/util/probe-event.c       |   25 ++++++++++++++++++-------
 tools/perf/util/trace-event-parse.c |   18 ++++++++----------
 6 files changed, 57 insertions(+), 20 deletions(-)

-- 
1.7.9.5

[PATCH 1/4] perf tools: remove sscanf extension %as

[Irina Tirdea]

From: Irina Tirdea <irina.tirdea@intel.com>

perf uses sscanf extension %as to read and allocate a
string in the same step. This is a non-standard extension
only present in new versions of glibc.

Replacing the use of sscanf and %as with strtok_r calls
in order to parse a given string into its components.
This is needed in Android since bionic does not support
%as extension for sscanf.

Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
---
 tools/perf/util/probe-event.c       |   25 ++++++++++++++++++-------
 tools/perf/util/trace-event-parse.c |   18 ++++++++----------
 2 files changed, 26 insertions(+), 17 deletions(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 4ce04c2..685ddcf 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -1100,6 +1100,7 @@ static int parse_probe_trace_command(const char *cmd,
 	struct probe_trace_point *tp = &tev->point;
 	char pr;
 	char *p;
+	char *argv0_str = NULL, *fmt, *fmt1_str, *fmt2_str, *fmt3_str;
 	int ret, i, argc;
 	char **argv;
 
@@ -1116,14 +1117,19 @@ static int parse_probe_trace_command(const char *cmd,
 	}
 
 	/* Scan event and group name. */
-	ret = sscanf(argv[0], "%c:%a[^/ \t]/%a[^ \t]",
-		     &pr, (float *)(void *)&tev->group,
-		     (float *)(void *)&tev->event);
-	if (ret != 3) {
+	argv0_str = strdup(argv[0]);
+	fmt1_str = strtok_r(argv0_str, ":", &fmt);
+	fmt2_str = strtok_r(NULL, "/", &fmt);
+	fmt3_str = strtok_r(NULL, " \t", &fmt);
+	if (fmt1_str == NULL || strlen(fmt1_str) != 1 || fmt2_str == NULL
+	    || fmt3_str == NULL) {
 		semantic_error("Failed to parse event name: %s\n", argv[0]);
 		ret = -EINVAL;
 		goto out;
 	}
+	pr = fmt1_str[0];
+	tev->group = strdup(fmt2_str);
+	tev->event = strdup(fmt3_str);
 	pr_debug("Group:%s Event:%s probe:%c\n", tev->group, tev->event, pr);
 
 	tp->retprobe = (pr == 'r');
@@ -1135,10 +1141,13 @@ static int parse_probe_trace_command(const char *cmd,
 		p++;
 	} else
 		p = argv[1];
-	ret = sscanf(p, "%a[^+]+%lu", (float *)(void *)&tp->symbol,
-		     &tp->offset);
-	if (ret == 1)
+	fmt1_str = strtok_r(p, "+", &fmt);
+	tp->symbol = strdup(fmt1_str);
+	fmt2_str = strtok_r(NULL, "", &fmt);
+	if (fmt2_str == NULL)
 		tp->offset = 0;
+	else
+		tp->offset = strtoul(fmt2_str, NULL, 10);
 
 	tev->nargs = argc - 2;
 	tev->args = zalloc(sizeof(struct probe_trace_arg) * tev->nargs);
@@ -1162,6 +1171,8 @@ static int parse_probe_trace_command(const char *cmd,
 	}
 	ret = 0;
 out:
+	if (argv0_str)
+		free(argv0_str);
 	argv_free(argv);
 	return ret;
 }
diff --git a/tools/perf/util/trace-event-parse.c b/tools/perf/util/trace-event-parse.c
index aa4c860..3aabcd6 100644
--- a/tools/perf/util/trace-event-parse.c
+++ b/tools/perf/util/trace-event-parse.c
@@ -229,24 +229,22 @@ void parse_proc_kallsyms(struct pevent *pevent,
 	char *next = NULL;
 	char *addr_str;
 	char *mod;
-	char ch;
+	char *fmt;
 
 	line = strtok_r(file, "\n", &next);
 	while (line) {
 		mod = NULL;
-		sscanf(line, "%as %c %as\t[%as",
-		       (float *)(void *)&addr_str, /* workaround gcc warning */
-		       &ch, (float *)(void *)&func, (float *)(void *)&mod);
+		addr_str = strtok_r(line, " ", &fmt);
 		addr = strtoull(addr_str, NULL, 16);
-		free(addr_str);
-
-		/* truncate the extra ']' */
+		/* skip character */
+		strtok_r(NULL, " ", &fmt);
+		func = strtok_r(NULL, "\t", &fmt);
+		mod = strtok_r(NULL, "]", &fmt);
+		/* truncate the extra '[' */
 		if (mod)
-			mod[strlen(mod) - 1] = 0;
+			mod = mod + 1;
 
 		pevent_register_function(pevent, func, addr, mod);
-		free(func);
-		free(mod);
 
 		line = strtok_r(NULL, "\n", &next);
 	}
-- 
1.7.9.5

[PATCH 2/4] perf stat: add compile-time option to disable --big-num

[Irina Tirdea]

From: Irina Tirdea <irina.tirdea@intel.com>

In printf's format, ' is used to group the output with thousands' grouping
characters for decimal conversion. Bionic does not support ' for printf.

Add a compile-time option (NO_BIG_NUM) to disable the --big-num option from
perf stat.

Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
---
 tools/perf/Makefile                 |    8 ++++++++
 tools/perf/builtin-stat.c           |    8 ++++++++
 tools/perf/config/feature-tests.mak |   12 ++++++++++++
 3 files changed, 28 insertions(+)

diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index 209774b..8daa781 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -768,6 +768,14 @@ else
        endif
 endif
 
+ifdef NO_BIG_NUM
+	BASIC_CFLAGS += -DNO_BIG_NUM
+else
+	ifneq ($(call try-cc,$(SOURCE_BIG_NUM),),y)
+		BASIC_CFLAGS += -DNO_BIG_NUM
+	endif
+endif
+
 ifdef ASCIIDOC8
 	export ASCIIDOC8
 endif
diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index dab347d..ad8013e 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -188,7 +188,11 @@ static pid_t			child_pid			= -1;
 static bool			null_run			=  false;
 static int			detailed_run			=  0;
 static bool			sync_run			=  false;
+#ifdef NO_BIG_NUM
+static bool			big_num				=  false;
+#else
 static bool			big_num				=  true;
+#endif
 static int			big_num_opt			=  -1;
 static const char		*csv_sep			= NULL;
 static bool			csv_output			= false;
@@ -1075,12 +1079,14 @@ static const char * const stat_usage[] = {
 	NULL
 };
 
+#ifndef NO_BIG_NUM
 static int stat__set_big_num(const struct option *opt __maybe_unused,
 			     const char *s __maybe_unused, int unset)
 {
 	big_num_opt = unset ? 0 : 1;
 	return 0;
 }
+#endif
 
 static bool append_file;
 
@@ -1112,9 +1118,11 @@ static const struct option options[] = {
 		    "detailed run - start a lot of events"),
 	OPT_BOOLEAN('S', "sync", &sync_run,
 		    "call sync() before starting a run"),
+#ifndef NO_BIG_NUM
 	OPT_CALLBACK_NOOPT('B', "big-num", NULL, NULL, 
 			   "print large numbers with thousands\' separators",
 			   stat__set_big_num),
+#endif
 	OPT_STRING('C', "cpu", &target.cpu_list, "cpu",
 		    "list of cpus to monitor in system-wide"),
 	OPT_BOOLEAN('A', "no-aggr", &no_aggr,
diff --git a/tools/perf/config/feature-tests.mak b/tools/perf/config/feature-tests.mak
index 116690a..2e5fad7 100644
--- a/tools/perf/config/feature-tests.mak
+++ b/tools/perf/config/feature-tests.mak
@@ -193,3 +193,15 @@ int main(void)
 }
 endef
 endif
+
+ifndef NO_BIG_NUM
+define SOURCE_BIG_NUM
+#include <stdio.h>
+
+int main(void)
+{
+	printf(\"%'\''.2f\", 1234567.89);
+	return 0;
+}
+endef
+endif
-- 
1.7.9.5

[PATCH 3/4] perf archive: remove -f from the rm command

[Irina Tirdea]

From: Irina Tirdea <irina.tirdea@intel.com>

In Android, rm does not support the -f parameter.
Remove -f from rm and make sure rm does not fail even if
the files to be removed are not found.

Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
---
 tools/perf/perf-archive.sh |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/perf/perf-archive.sh b/tools/perf/perf-archive.sh
index 95b6f8b..da94179 100644
--- a/tools/perf/perf-archive.sh
+++ b/tools/perf/perf-archive.sh
@@ -24,7 +24,7 @@ NOBUILDID=0000000000000000000000000000000000000000
 perf buildid-list -i $PERF_DATA --with-hits | grep -v "^$NOBUILDID " > $BUILDIDS
 if [ ! -s $BUILDIDS ] ; then
 	echo "perf archive: no build-ids found"
-	rm -f $BUILDIDS
+	rm $BUILDIDS || true
 	exit 1
 fi
 
@@ -40,7 +40,7 @@ while read build_id ; do
 done
 
 tar cfj $PERF_DATA.tar.bz2 -C $PERF_BUILDID_DIR -T $MANIFEST
-rm -f $MANIFEST $BUILDIDS
+rm $MANIFEST $BUILDIDS || true
 echo -e "Now please run:\n"
 echo -e "$ tar xvf $PERF_DATA.tar.bz2 -C ~/.debug\n"
 echo "wherever you need to run 'perf report' on."
-- 
1.7.9.5

[PATCH 4/4] perf archive: make f the last parameter for tar

[Irina Tirdea]

From: Irina Tirdea <irina.tirdea@intel.com>

On some systems, tar needs to specify the name of the archive immediately
after the -f parameter.

Change the order of the parameters so tar can run properly.

Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
---
 tools/perf/perf-archive.sh |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/perf-archive.sh b/tools/perf/perf-archive.sh
index da94179..e919306 100644
--- a/tools/perf/perf-archive.sh
+++ b/tools/perf/perf-archive.sh
@@ -39,7 +39,7 @@ while read build_id ; do
 	echo ${filename#$PERF_BUILDID_LINKDIR} >> $MANIFEST
 done
 
-tar cfj $PERF_DATA.tar.bz2 -C $PERF_BUILDID_DIR -T $MANIFEST
+tar cjf $PERF_DATA.tar.bz2 -C $PERF_BUILDID_DIR -T $MANIFEST
 rm $MANIFEST $BUILDIDS || true
 echo -e "Now please run:\n"
 echo -e "$ tar xvf $PERF_DATA.tar.bz2 -C ~/.debug\n"
-- 
1.7.9.5

Re: [PATCH 1/4] perf tools: remove sscanf extension %as

[Namhyung Kim]

Hi Irina,

(Adding Masami to Cc)

On Fri, 14 Sep 2012 01:07:40 +0300, Irina Tirdea wrote:
> From: Irina Tirdea <irina.tirdea@intel.com>
>
> perf uses sscanf extension %as to read and allocate a
> string in the same step. This is a non-standard extension
> only present in new versions of glibc.
>
> Replacing the use of sscanf and %as with strtok_r calls
> in order to parse a given string into its components.
> This is needed in Android since bionic does not support
> %as extension for sscanf.
>
> Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
> ---
>  tools/perf/util/probe-event.c       |   25 ++++++++++++++++++-------
>  tools/perf/util/trace-event-parse.c |   18 ++++++++----------
>  2 files changed, 26 insertions(+), 17 deletions(-)
>
> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> index 4ce04c2..685ddcf 100644
> --- a/tools/perf/util/probe-event.c
> +++ b/tools/perf/util/probe-event.c
> @@ -1100,6 +1100,7 @@ static int parse_probe_trace_command(const char *cmd,
>  	struct probe_trace_point *tp = &tev->point;
>  	char pr;
>  	char *p;
> +	char *argv0_str = NULL, *fmt, *fmt1_str, *fmt2_str, *fmt3_str;
>  	int ret, i, argc;
>  	char **argv;
>  
> @@ -1116,14 +1117,19 @@ static int parse_probe_trace_command(const char *cmd,
>  	}
>  
>  	/* Scan event and group name. */
> -	ret = sscanf(argv[0], "%c:%a[^/ \t]/%a[^ \t]",
> -		     &pr, (float *)(void *)&tev->group,
> -		     (float *)(void *)&tev->event);
> -	if (ret != 3) {
> +	argv0_str = strdup(argv[0]);

It seems you need to check return value of strdup.


> +	fmt1_str = strtok_r(argv0_str, ":", &fmt);
> +	fmt2_str = strtok_r(NULL, "/", &fmt);
> +	fmt3_str = strtok_r(NULL, " \t", &fmt);
> +	if (fmt1_str == NULL || strlen(fmt1_str) != 1 || fmt2_str == NULL
> +	    || fmt3_str == NULL) {
>  		semantic_error("Failed to parse event name: %s\n", argv[0]);
>  		ret = -EINVAL;
>  		goto out;
>  	}
> +	pr = fmt1_str[0];
> +	tev->group = strdup(fmt2_str);
> +	tev->event = strdup(fmt3_str);
>  	pr_debug("Group:%s Event:%s probe:%c\n", tev->group, tev->event, pr);
>  
>  	tp->retprobe = (pr == 'r');
> @@ -1135,10 +1141,13 @@ static int parse_probe_trace_command(const char *cmd,
>  		p++;
>  	} else
>  		p = argv[1];
> -	ret = sscanf(p, "%a[^+]+%lu", (float *)(void *)&tp->symbol,
> -		     &tp->offset);
> -	if (ret == 1)
> +	fmt1_str = strtok_r(p, "+", &fmt);
> +	tp->symbol = strdup(fmt1_str);

Probably here too - although the original code didn't but I think it's
needed.


> +	fmt2_str = strtok_r(NULL, "", &fmt);
> +	if (fmt2_str == NULL)
>  		tp->offset = 0;
> +	else
> +		tp->offset = strtoul(fmt2_str, NULL, 10);
>  
>  	tev->nargs = argc - 2;
>  	tev->args = zalloc(sizeof(struct probe_trace_arg) * tev->nargs);
> @@ -1162,6 +1171,8 @@ static int parse_probe_trace_command(const char *cmd,
>  	}
>  	ret = 0;
>  out:
> +	if (argv0_str)
> +		free(argv0_str);

The free() can handle a NULL pointer safely.


>  	argv_free(argv);
>  	return ret;
>  }

Re: [PATCH 2/4] perf stat: add compile-time option to disable --big-num

[Ingo Molnar]

* Irina Tirdea <irina.tirdea@gmail.com> wrote:

> From: Irina Tirdea <irina.tirdea@intel.com>
> 
> In printf's format, ' is used to group the output with thousands' grouping
> characters for decimal conversion. Bionic does not support ' for printf.

Please try to solve compatibility without affecting the default 
output and big num is the default perf stat output.

Could the big num output be implemented within perf, without 
relying on glibc's printf implementation?

Thanks,

	Ingo

Re: [PATCH 1/4] perf tools: remove sscanf extension %as

[Masami Hiramatsu]

(2012/09/14 10:54), Namhyung Kim wrote:
> Hi Irina,
> 
> (Adding Masami to Cc)

Thanks Irina and Namhyung :)

> On Fri, 14 Sep 2012 01:07:40 +0300, Irina Tirdea wrote:
>> From: Irina Tirdea <irina.tirdea@intel.com>
>>
>> perf uses sscanf extension %as to read and allocate a
>> string in the same step. This is a non-standard extension
>> only present in new versions of glibc.
>>
>> Replacing the use of sscanf and %as with strtok_r calls
>> in order to parse a given string into its components.
>> This is needed in Android since bionic does not support
>> %as extension for sscanf.
>>
>> Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
>> ---
>>  tools/perf/util/probe-event.c       |   25 ++++++++++++++++++-------
>>  tools/perf/util/trace-event-parse.c |   18 ++++++++----------
>>  2 files changed, 26 insertions(+), 17 deletions(-)
>>
>> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
>> index 4ce04c2..685ddcf 100644
>> --- a/tools/perf/util/probe-event.c
>> +++ b/tools/perf/util/probe-event.c
>> @@ -1100,6 +1100,7 @@ static int parse_probe_trace_command(const char *cmd,
>>  	struct probe_trace_point *tp = &tev->point;
>>  	char pr;
>>  	char *p;
>> +	char *argv0_str = NULL, *fmt, *fmt1_str, *fmt2_str, *fmt3_str;
>>  	int ret, i, argc;
>>  	char **argv;
>>  
>> @@ -1116,14 +1117,19 @@ static int parse_probe_trace_command(const char *cmd,
>>  	}
>>  
>>  	/* Scan event and group name. */
>> -	ret = sscanf(argv[0], "%c:%a[^/ \t]/%a[^ \t]",
>> -		     &pr, (float *)(void *)&tev->group,
>> -		     (float *)(void *)&tev->event);
>> -	if (ret != 3) {
>> +	argv0_str = strdup(argv[0]);
> 
> It seems you need to check return value of strdup.

Agreed.

>> +	fmt1_str = strtok_r(argv0_str, ":", &fmt);
>> +	fmt2_str = strtok_r(NULL, "/", &fmt);
>> +	fmt3_str = strtok_r(NULL, " \t", &fmt);
>> +	if (fmt1_str == NULL || strlen(fmt1_str) != 1 || fmt2_str == NULL
>> +	    || fmt3_str == NULL) {
>>  		semantic_error("Failed to parse event name: %s\n", argv[0]);
>>  		ret = -EINVAL;
>>  		goto out;
>>  	}
>> +	pr = fmt1_str[0];
>> +	tev->group = strdup(fmt2_str);
>> +	tev->event = strdup(fmt3_str);
>>  	pr_debug("Group:%s Event:%s probe:%c\n", tev->group, tev->event, pr);
>>  
>>  	tp->retprobe = (pr == 'r');
>> @@ -1135,10 +1141,13 @@ static int parse_probe_trace_command(const char *cmd,
>>  		p++;
>>  	} else
>>  		p = argv[1];
>> -	ret = sscanf(p, "%a[^+]+%lu", (float *)(void *)&tp->symbol,
>> -		     &tp->offset);
>> -	if (ret == 1)
>> +	fmt1_str = strtok_r(p, "+", &fmt);
>> +	tp->symbol = strdup(fmt1_str);
> 
> Probably here too - although the original code didn't but I think it's
> needed.

right, it should be fixed.

>> +	fmt2_str = strtok_r(NULL, "", &fmt);
>> +	if (fmt2_str == NULL)
>>  		tp->offset = 0;
>> +	else
>> +		tp->offset = strtoul(fmt2_str, NULL, 10);
>>  
>>  	tev->nargs = argc - 2;
>>  	tev->args = zalloc(sizeof(struct probe_trace_arg) * tev->nargs);
>> @@ -1162,6 +1171,8 @@ static int parse_probe_trace_command(const char *cmd,
>>  	}
>>  	ret = 0;
>>  out:
>> +	if (argv0_str)
>> +		free(argv0_str);
> 
> The free() can handle a NULL pointer safely.

I don't care about it, since there are already lots of "if (ptr) free(ptr)"
style code ;)

> 
>>  	argv_free(argv);
>>  	return ret;
>>  }

Thank you!

-- 
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com

[tip:perf/core] perf archive: Remove -f from the rm command

[tip-bot for Irina Tirdea]

Commit-ID:  73eb422c10aa2a4afc1100d1bd54974ed72ad373
Gitweb:     http://git.kernel.org/tip/73eb422c10aa2a4afc1100d1bd54974ed72ad373
Author:     Irina Tirdea <irina.tirdea@intel.com>
AuthorDate: Fri, 14 Sep 2012 01:07:42 +0300
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 17 Sep 2012 13:10:27 -0300

perf archive: Remove -f from the rm command

In Android, rm does not support the -f parameter.

Remove -f from rm and make sure rm does not fail even if the files to be
removed are not found.

Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Namhyung Kim <namhyung.kim@lge.com>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Steven Rostedt <rostedt@goodmis.org>
Link: http://lkml.kernel.org/r/1347574063-22521-4-git-send-email-irina.tirdea@intel.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/perf-archive.sh |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/perf/perf-archive.sh b/tools/perf/perf-archive.sh
index 95b6f8b..da94179 100644
--- a/tools/perf/perf-archive.sh
+++ b/tools/perf/perf-archive.sh
@@ -24,7 +24,7 @@ NOBUILDID=0000000000000000000000000000000000000000
 perf buildid-list -i $PERF_DATA --with-hits | grep -v "^$NOBUILDID " > $BUILDIDS
 if [ ! -s $BUILDIDS ] ; then
 	echo "perf archive: no build-ids found"
-	rm -f $BUILDIDS
+	rm $BUILDIDS || true
 	exit 1
 fi
 
@@ -40,7 +40,7 @@ while read build_id ; do
 done
 
 tar cfj $PERF_DATA.tar.bz2 -C $PERF_BUILDID_DIR -T $MANIFEST
-rm -f $MANIFEST $BUILDIDS
+rm $MANIFEST $BUILDIDS || true
 echo -e "Now please run:\n"
 echo -e "$ tar xvf $PERF_DATA.tar.bz2 -C ~/.debug\n"
 echo "wherever you need to run 'perf report' on."

[tip:perf/core] perf archive: Make 'f' the last parameter for tar

[tip-bot for Irina Tirdea]

Commit-ID:  87ff50a3192152944d8e65b485bbf3d03214d0b6
Gitweb:     http://git.kernel.org/tip/87ff50a3192152944d8e65b485bbf3d03214d0b6
Author:     Irina Tirdea <irina.tirdea@intel.com>
AuthorDate: Fri, 14 Sep 2012 01:07:43 +0300
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 17 Sep 2012 13:10:42 -0300

perf archive: Make 'f' the last parameter for tar

On some systems, tar needs to specify the name of the archive immediately
after the -f parameter.

Change the order of the parameters so tar can run properly.

Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Namhyung Kim <namhyung.kim@lge.com>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Steven Rostedt <rostedt@goodmis.org>
Link: http://lkml.kernel.org/r/1347574063-22521-5-git-send-email-irina.tirdea@intel.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/perf-archive.sh |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/tools/perf/perf-archive.sh b/tools/perf/perf-archive.sh
index da94179..e919306 100644
--- a/tools/perf/perf-archive.sh
+++ b/tools/perf/perf-archive.sh
@@ -39,7 +39,7 @@ while read build_id ; do
 	echo ${filename#$PERF_BUILDID_LINKDIR} >> $MANIFEST
 done
 
-tar cfj $PERF_DATA.tar.bz2 -C $PERF_BUILDID_DIR -T $MANIFEST
+tar cjf $PERF_DATA.tar.bz2 -C $PERF_BUILDID_DIR -T $MANIFEST
 rm $MANIFEST $BUILDIDS || true
 echo -e "Now please run:\n"
 echo -e "$ tar xvf $PERF_DATA.tar.bz2 -C ~/.debug\n"

Re: [PATCH 1/4] perf tools: remove sscanf extension %as

[Irina Tirdea]

>> On Fri, 14 Sep 2012 01:07:40 +0300, Irina Tirdea wrote:
>>> From: Irina Tirdea <irina.tirdea@intel.com>
>>>
>>> perf uses sscanf extension %as to read and allocate a
>>> string in the same step. This is a non-standard extension
>>> only present in new versions of glibc.
>>>
>>> Replacing the use of sscanf and %as with strtok_r calls
>>> in order to parse a given string into its components.
>>> This is needed in Android since bionic does not support
>>> %as extension for sscanf.
>>>
>>> Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
>>> ---
>>>  tools/perf/util/probe-event.c       |   25 ++++++++++++++++++-------
>>>  tools/perf/util/trace-event-parse.c |   18 ++++++++----------
>>>  2 files changed, 26 insertions(+), 17 deletions(-)
>>>
>>> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
>>> index 4ce04c2..685ddcf 100644
>>> --- a/tools/perf/util/probe-event.c
>>> +++ b/tools/perf/util/probe-event.c
>>> @@ -1100,6 +1100,7 @@ static int parse_probe_trace_command(const char *cmd,
>>>      struct probe_trace_point *tp = &tev->point;
>>>      char pr;
>>>      char *p;
>>> +    char *argv0_str = NULL, *fmt, *fmt1_str, *fmt2_str, *fmt3_str;
>>>      int ret, i, argc;
>>>      char **argv;
>>>
>>> @@ -1116,14 +1117,19 @@ static int parse_probe_trace_command(const char *cmd,
>>>      }
>>>
>>>      /* Scan event and group name. */
>>> -    ret = sscanf(argv[0], "%c:%a[^/ \t]/%a[^ \t]",
>>> -                 &pr, (float *)(void *)&tev->group,
>>> -                 (float *)(void *)&tev->event);
>>> -    if (ret != 3) {
>>> +    argv0_str = strdup(argv[0]);
>>
>> It seems you need to check return value of strdup.
>
> Agreed.


Thanks for the review! I'll make the fixes and resend the patch.

Irina

Re: [PATCH 2/4] perf stat: add compile-time option to disable --big-num

[Irina Tirdea]

On Fri, Sep 14, 2012 at 8:40 AM, Ingo Molnar <mingo@kernel.org> wrote:
>
> * Irina Tirdea <irina.tirdea@gmail.com> wrote:
>
>> From: Irina Tirdea <irina.tirdea@intel.com>
>>
>> In printf's format, ' is used to group the output with thousands' grouping
>> characters for decimal conversion. Bionic does not support ' for printf.
>
> Please try to solve compatibility without affecting the default
> output and big num is the default perf stat output.

Understood.

> Could the big num output be implemented within perf, without
> relying on glibc's printf implementation?
>

Yes. I'll write a function that does the grouping instead of using
printf and send it for review instead of this patch.

Thanks,
Irina

[PATCH v2 1/1] perf tools: remove sscanf extension %as

[irina.tirdea]

From: Irina Tirdea <irina.tirdea@intel.com>

perf uses sscanf extension %as to read and allocate a
string in the same step. This is a non-standard extension
only present in new versions of glibc.

Replacing the use of sscanf and %as with strtok_r calls
in order to parse a given string into its components.
This is needed in Android since bionic does not support
%as extension for sscanf.

Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
---
 tools/perf/util/probe-event.c       |   36 ++++++++++++++++++++++++++++-------
 tools/perf/util/trace-event-parse.c |   18 ++++++++----------
 2 files changed, 37 insertions(+), 17 deletions(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 4ce04c2..49a256e 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -1100,6 +1100,7 @@ static int parse_probe_trace_command(const char *cmd,
 	struct probe_trace_point *tp = &tev->point;
 	char pr;
 	char *p;
+	char *argv0_str = NULL, *fmt, *fmt1_str, *fmt2_str, *fmt3_str;
 	int ret, i, argc;
 	char **argv;
 
@@ -1116,14 +1117,27 @@ static int parse_probe_trace_command(const char *cmd,
 	}
 
 	/* Scan event and group name. */
-	ret = sscanf(argv[0], "%c:%a[^/ \t]/%a[^ \t]",
-		     &pr, (float *)(void *)&tev->group,
-		     (float *)(void *)&tev->event);
-	if (ret != 3) {
+	argv0_str = strdup(argv[0]);
+	if (argv0_str == NULL) {
+		ret = -ENOMEM;
+		goto out;
+	}
+	fmt1_str = strtok_r(argv0_str, ":", &fmt);
+	fmt2_str = strtok_r(NULL, "/", &fmt);
+	fmt3_str = strtok_r(NULL, " \t", &fmt);
+	if (fmt1_str == NULL || strlen(fmt1_str) != 1 || fmt2_str == NULL
+	    || fmt3_str == NULL) {
 		semantic_error("Failed to parse event name: %s\n", argv[0]);
 		ret = -EINVAL;
 		goto out;
 	}
+	pr = fmt1_str[0];
+	tev->group = strdup(fmt2_str);
+	tev->event = strdup(fmt3_str);
+	if (tev->group == NULL || tev->event == NULL) {
+		ret = -ENOMEM;
+		goto out;
+	}
 	pr_debug("Group:%s Event:%s probe:%c\n", tev->group, tev->event, pr);
 
 	tp->retprobe = (pr == 'r');
@@ -1135,10 +1149,17 @@ static int parse_probe_trace_command(const char *cmd,
 		p++;
 	} else
 		p = argv[1];
-	ret = sscanf(p, "%a[^+]+%lu", (float *)(void *)&tp->symbol,
-		     &tp->offset);
-	if (ret == 1)
+	fmt1_str = strtok_r(p, "+", &fmt);
+	tp->symbol = strdup(fmt1_str);
+	if (tp->symbol == NULL) {
+		ret = -ENOMEM;
+		goto out;
+	}
+	fmt2_str = strtok_r(NULL, "", &fmt);
+	if (fmt2_str == NULL)
 		tp->offset = 0;
+	else
+		tp->offset = strtoul(fmt2_str, NULL, 10);
 
 	tev->nargs = argc - 2;
 	tev->args = zalloc(sizeof(struct probe_trace_arg) * tev->nargs);
@@ -1162,6 +1183,7 @@ static int parse_probe_trace_command(const char *cmd,
 	}
 	ret = 0;
 out:
+	free(argv0_str);
 	argv_free(argv);
 	return ret;
 }
diff --git a/tools/perf/util/trace-event-parse.c b/tools/perf/util/trace-event-parse.c
index aa4c860..3aabcd6 100644
--- a/tools/perf/util/trace-event-parse.c
+++ b/tools/perf/util/trace-event-parse.c
@@ -229,24 +229,22 @@ void parse_proc_kallsyms(struct pevent *pevent,
 	char *next = NULL;
 	char *addr_str;
 	char *mod;
-	char ch;
+	char *fmt;
 
 	line = strtok_r(file, "\n", &next);
 	while (line) {
 		mod = NULL;
-		sscanf(line, "%as %c %as\t[%as",
-		       (float *)(void *)&addr_str, /* workaround gcc warning */
-		       &ch, (float *)(void *)&func, (float *)(void *)&mod);
+		addr_str = strtok_r(line, " ", &fmt);
 		addr = strtoull(addr_str, NULL, 16);
-		free(addr_str);
-
-		/* truncate the extra ']' */
+		/* skip character */
+		strtok_r(NULL, " ", &fmt);
+		func = strtok_r(NULL, "\t", &fmt);
+		mod = strtok_r(NULL, "]", &fmt);
+		/* truncate the extra '[' */
 		if (mod)
-			mod[strlen(mod) - 1] = 0;
+			mod = mod + 1;
 
 		pevent_register_function(pevent, func, addr, mod);
-		free(func);
-		free(mod);
 
 		line = strtok_r(NULL, "\n", &next);
 	}
-- 
1.7.9.5

Re: [PATCH v2 1/1] perf tools: remove sscanf extension %as

[Arnaldo Carvalho de Melo]

Em Thu, Sep 20, 2012 at 11:37:50PM +0300, irina.tirdea@gmail.com escreveu:
> From: Irina Tirdea <irina.tirdea@intel.com>
> 
> perf uses sscanf extension %as to read and allocate a
> string in the same step. This is a non-standard extension
> only present in new versions of glibc.
> 
> Replacing the use of sscanf and %as with strtok_r calls
> in order to parse a given string into its components.
> This is needed in Android since bionic does not support
> %as extension for sscanf.

Masami, can I have your Acked-by or even a Tested-by?

- Arnaldo
 
> Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
> ---
>  tools/perf/util/probe-event.c       |   36 ++++++++++++++++++++++++++++-------
>  tools/perf/util/trace-event-parse.c |   18 ++++++++----------
>  2 files changed, 37 insertions(+), 17 deletions(-)
> 
> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> index 4ce04c2..49a256e 100644
> --- a/tools/perf/util/probe-event.c
> +++ b/tools/perf/util/probe-event.c
> @@ -1100,6 +1100,7 @@ static int parse_probe_trace_command(const char *cmd,
>  	struct probe_trace_point *tp = &tev->point;
>  	char pr;
>  	char *p;
> +	char *argv0_str = NULL, *fmt, *fmt1_str, *fmt2_str, *fmt3_str;
>  	int ret, i, argc;
>  	char **argv;
>  
> @@ -1116,14 +1117,27 @@ static int parse_probe_trace_command(const char *cmd,
>  	}
>  
>  	/* Scan event and group name. */
> -	ret = sscanf(argv[0], "%c:%a[^/ \t]/%a[^ \t]",
> -		     &pr, (float *)(void *)&tev->group,
> -		     (float *)(void *)&tev->event);
> -	if (ret != 3) {
> +	argv0_str = strdup(argv[0]);
> +	if (argv0_str == NULL) {
> +		ret = -ENOMEM;
> +		goto out;
> +	}
> +	fmt1_str = strtok_r(argv0_str, ":", &fmt);
> +	fmt2_str = strtok_r(NULL, "/", &fmt);
> +	fmt3_str = strtok_r(NULL, " \t", &fmt);
> +	if (fmt1_str == NULL || strlen(fmt1_str) != 1 || fmt2_str == NULL
> +	    || fmt3_str == NULL) {
>  		semantic_error("Failed to parse event name: %s\n", argv[0]);
>  		ret = -EINVAL;
>  		goto out;
>  	}
> +	pr = fmt1_str[0];
> +	tev->group = strdup(fmt2_str);
> +	tev->event = strdup(fmt3_str);
> +	if (tev->group == NULL || tev->event == NULL) {
> +		ret = -ENOMEM;
> +		goto out;
> +	}
>  	pr_debug("Group:%s Event:%s probe:%c\n", tev->group, tev->event, pr);
>  
>  	tp->retprobe = (pr == 'r');
> @@ -1135,10 +1149,17 @@ static int parse_probe_trace_command(const char *cmd,
>  		p++;
>  	} else
>  		p = argv[1];
> -	ret = sscanf(p, "%a[^+]+%lu", (float *)(void *)&tp->symbol,
> -		     &tp->offset);
> -	if (ret == 1)
> +	fmt1_str = strtok_r(p, "+", &fmt);
> +	tp->symbol = strdup(fmt1_str);
> +	if (tp->symbol == NULL) {
> +		ret = -ENOMEM;
> +		goto out;
> +	}
> +	fmt2_str = strtok_r(NULL, "", &fmt);
> +	if (fmt2_str == NULL)
>  		tp->offset = 0;
> +	else
> +		tp->offset = strtoul(fmt2_str, NULL, 10);
>  
>  	tev->nargs = argc - 2;
>  	tev->args = zalloc(sizeof(struct probe_trace_arg) * tev->nargs);
> @@ -1162,6 +1183,7 @@ static int parse_probe_trace_command(const char *cmd,
>  	}
>  	ret = 0;
>  out:
> +	free(argv0_str);
>  	argv_free(argv);
>  	return ret;
>  }
> diff --git a/tools/perf/util/trace-event-parse.c b/tools/perf/util/trace-event-parse.c
> index aa4c860..3aabcd6 100644
> --- a/tools/perf/util/trace-event-parse.c
> +++ b/tools/perf/util/trace-event-parse.c
> @@ -229,24 +229,22 @@ void parse_proc_kallsyms(struct pevent *pevent,
>  	char *next = NULL;
>  	char *addr_str;
>  	char *mod;
> -	char ch;
> +	char *fmt;
>  
>  	line = strtok_r(file, "\n", &next);
>  	while (line) {
>  		mod = NULL;
> -		sscanf(line, "%as %c %as\t[%as",
> -		       (float *)(void *)&addr_str, /* workaround gcc warning */
> -		       &ch, (float *)(void *)&func, (float *)(void *)&mod);
> +		addr_str = strtok_r(line, " ", &fmt);
>  		addr = strtoull(addr_str, NULL, 16);
> -		free(addr_str);
> -
> -		/* truncate the extra ']' */
> +		/* skip character */
> +		strtok_r(NULL, " ", &fmt);
> +		func = strtok_r(NULL, "\t", &fmt);
> +		mod = strtok_r(NULL, "]", &fmt);
> +		/* truncate the extra '[' */
>  		if (mod)
> -			mod[strlen(mod) - 1] = 0;
> +			mod = mod + 1;
>  
>  		pevent_register_function(pevent, func, addr, mod);
> -		free(func);
> -		free(mod);
>  
>  		line = strtok_r(NULL, "\n", &next);
>  	}
> -- 
> 1.7.9.5

[PATCH v2 1/1] perf stat: implement --big-num grouping

[Irina Tirdea]

From: Irina Tirdea <irina.tirdea@intel.com>

In glibc, printf supports ' to group numbers with thousands' grouping
characters. Bionic does not support ' for printf.

Implement thousands's grouping for numbers according to locale.
The implementation uses the algorithm from glibc
(http://www.gnu.org/software/libc/).

Bionic does not implement locales, so we need to add a configuration
option NO_LOCALE. If NO_LOCALE is defined, default values for thousands
separator and grouping are used.

Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
---

Changes in v2:
This is a rewrite of http://lkml.org/lkml/2012/9/13/574
Instead of disabling big num for Android, I added the implementation in perf
(as Ingo suggested).

 tools/perf/Makefile                 |    8 +++
 tools/perf/builtin-stat.c           |  112 ++++++++++++++++++++++++++++++++---
 tools/perf/config/feature-tests.mak |   18 ++++++
 3 files changed, 131 insertions(+), 7 deletions(-)

diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index 5077f8e..74e21cf 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -769,6 +769,14 @@ else
        endif
 endif
 
+ifdef NO_LOCALE
+	BASIC_CFLAGS += -DNO_LOCALE
+else
+	ifneq ($(call try-cc,$(SOURCE_LOCALE),),y)
+		BASIC_CFLAGS += -DNO_LOCALE
+	endif
+endif
+
 ifdef ASCIIDOC8
 	export ASCIIDOC8
 endif
diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index e0f65fe..cb8b399 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -60,6 +60,8 @@
 #include <sys/prctl.h>
 #include <locale.h>
 
+/* max double number have E+308 + \0 + sign */
+#define MAX_NR_STR 310
 #define DEFAULT_SEPARATOR	" "
 #define CNTR_NOT_SUPPORTED	"<not supported>"
 #define CNTR_NOT_COUNTED	"<not counted>"
@@ -744,18 +746,112 @@ static void print_ll_cache_misses(int cpu,
 	fprintf(output, " of all LL-cache hits   ");
 }
 
+/* Group the digits according to the grouping rules of the current locale.
+   The interpretation of GROUPING is as in `struct lconv' from <locale.h>.  */
+static int group_number_locale(char *number, char **gnumber)
+{
+	const char *thousands_sep = NULL, *grouping = NULL;
+	int glen, tlen, dest_alloc_size, src_size, ret = 0, cnt;
+	char *dest_alloc_ptr, *dest_end, *src_start, *src_end;
+
+#ifdef NO_LOCALE
+	thousands_sep = ",";
+	grouping = "\x3";
+#else
+	struct lconv *lc = localeconv();
+	if (lc != NULL) {
+		thousands_sep = lc->thousands_sep;
+		grouping = lc->grouping;
+	}
+#endif
+
+	*gnumber = NULL;
+	/* No grouping */
+	if (thousands_sep == NULL || grouping == NULL ||
+	    *thousands_sep == '\0' || *grouping == CHAR_MAX || *grouping <= 0) {
+		*gnumber = strdup(number);
+		if (*gnumber == NULL)
+			ret = -ENOMEM;
+		goto out;
+	}
+
+	glen = *grouping++;
+	tlen = strlen(thousands_sep);
+
+	src_size = strlen(number);
+	/* Worst case scenario we have 1-character grouping */
+	dest_alloc_size = (src_size + src_size * tlen) * sizeof(char);
+	dest_alloc_ptr = zalloc(dest_alloc_size);
+	if (dest_alloc_ptr == NULL) {
+		ret = -ENOMEM;
+		goto out;
+	}
+	/* -1 for '\0' */
+	dest_end = dest_alloc_ptr + dest_alloc_size - 1;
+
+	src_start = number;
+	src_end = number + src_size;
+
+	while (src_end > src_start) {
+		*--dest_end = *--src_end;
+		if (--glen == 0 && src_end > src_start) {
+			/* A new group */
+			cnt = tlen;
+			do
+				*--dest_end = thousands_sep[--cnt];
+			while (cnt > 0);
+
+			if (*grouping == CHAR_MAX || *grouping < 0) {
+				/* No further grouping to be done.
+				   Copy the rest of the number. */
+				do
+					*--dest_end = *--src_end;
+				while (src_end > src_start);
+				break;
+			} else if (*grouping != '\0') {
+				glen = *grouping++;
+			} else {
+				/* The previous grouping repeats ad infinitum */
+				glen = grouping[-1];
+			}
+		}
+	}
+
+	/* Make a copy with the exact needed size of the grouped number */
+	*gnumber = strdup(dest_end);
+	if (*gnumber == NULL) {
+		ret = -ENOMEM;
+		goto out_free_dest;
+	}
+
+	/* fall through */
+out_free_dest:
+	free(dest_alloc_ptr);
+out:
+	return ret;
+}
+
 static void abs_printout(int cpu, struct perf_evsel *evsel, double avg)
 {
 	double total, ratio = 0.0;
 	char cpustr[16] = { '\0', };
 	const char *fmt;
+	char avgstr[MAX_NR_STR], *pavgstr;
+	int ret;
 
-	if (csv_output)
-		fmt = "%s%.0f%s%s";
-	else if (big_num)
-		fmt = "%s%'18.0f%s%-25s";
-	else
-		fmt = "%s%18.0f%s%-25s";
+	sprintf(avgstr, "%.0f", avg);
+	pavgstr = avgstr;
+
+	if (csv_output) {
+		fmt = "%s%s%s%s";
+	} else {
+		fmt = "%s%18s%s%-25s";
+		if (big_num) {
+			ret = group_number_locale(avgstr, &pavgstr);
+			if (ret < 0)
+				pavgstr = avgstr;
+		}
+	}
 
 	if (no_aggr)
 		sprintf(cpustr, "CPU%*d%s",
@@ -764,7 +860,9 @@ static void abs_printout(int cpu, struct perf_evsel *evsel, double avg)
 	else
 		cpu = 0;
 
-	fprintf(output, fmt, cpustr, avg, csv_sep, perf_evsel__name(evsel));
+	fprintf(output, fmt, cpustr, pavgstr, csv_sep, perf_evsel__name(evsel));
+	if (pavgstr != avgstr)
+		free(pavgstr);
 
 	if (evsel->cgrp)
 		fprintf(output, "%s%s", csv_sep, evsel->cgrp->name);
diff --git a/tools/perf/config/feature-tests.mak b/tools/perf/config/feature-tests.mak
index 116690a..7e5c005 100644
--- a/tools/perf/config/feature-tests.mak
+++ b/tools/perf/config/feature-tests.mak
@@ -193,3 +193,21 @@ int main(void)
 }
 endef
 endif
+
+ifndef NO_LOCALE
+define SOURCE_LOCALE
+#include <locale.h>
+
+int main(void)
+{
+	char *thousands_sep, *grouping;
+
+	struct lconv *lc = localeconv();
+	if (lc != NULL) {
+		thousands_sep = lc->thousands_sep;
+		grouping = lc->grouping;
+	}
+	return 0;
+}
+endef
+endif
-- 
1.7.9.5

Re: [PATCH v2 1/1] perf tools: remove sscanf extension %as

[Masami Hiramatsu]

(2012/09/21 5:37), irina.tirdea@gmail.com wrote:
> From: Irina Tirdea <irina.tirdea@intel.com>
> 
> perf uses sscanf extension %as to read and allocate a
> string in the same step. This is a non-standard extension
> only present in new versions of glibc.
> 
> Replacing the use of sscanf and %as with strtok_r calls
> in order to parse a given string into its components.
> This is needed in Android since bionic does not support
> %as extension for sscanf.

OK, I've reviewed and tested, looks good! :)

Acked-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>

Thank you!

> 
> Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
> ---
>  tools/perf/util/probe-event.c       |   36 ++++++++++++++++++++++++++++-------
>  tools/perf/util/trace-event-parse.c |   18 ++++++++----------
>  2 files changed, 37 insertions(+), 17 deletions(-)
> 
> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> index 4ce04c2..49a256e 100644
> --- a/tools/perf/util/probe-event.c
> +++ b/tools/perf/util/probe-event.c
> @@ -1100,6 +1100,7 @@ static int parse_probe_trace_command(const char *cmd,
>  	struct probe_trace_point *tp = &tev->point;
>  	char pr;
>  	char *p;
> +	char *argv0_str = NULL, *fmt, *fmt1_str, *fmt2_str, *fmt3_str;
>  	int ret, i, argc;
>  	char **argv;
>  
> @@ -1116,14 +1117,27 @@ static int parse_probe_trace_command(const char *cmd,
>  	}
>  
>  	/* Scan event and group name. */
> -	ret = sscanf(argv[0], "%c:%a[^/ \t]/%a[^ \t]",
> -		     &pr, (float *)(void *)&tev->group,
> -		     (float *)(void *)&tev->event);
> -	if (ret != 3) {
> +	argv0_str = strdup(argv[0]);
> +	if (argv0_str == NULL) {
> +		ret = -ENOMEM;
> +		goto out;
> +	}
> +	fmt1_str = strtok_r(argv0_str, ":", &fmt);
> +	fmt2_str = strtok_r(NULL, "/", &fmt);
> +	fmt3_str = strtok_r(NULL, " \t", &fmt);
> +	if (fmt1_str == NULL || strlen(fmt1_str) != 1 || fmt2_str == NULL
> +	    || fmt3_str == NULL) {
>  		semantic_error("Failed to parse event name: %s\n", argv[0]);
>  		ret = -EINVAL;
>  		goto out;
>  	}
> +	pr = fmt1_str[0];
> +	tev->group = strdup(fmt2_str);
> +	tev->event = strdup(fmt3_str);
> +	if (tev->group == NULL || tev->event == NULL) {
> +		ret = -ENOMEM;
> +		goto out;
> +	}
>  	pr_debug("Group:%s Event:%s probe:%c\n", tev->group, tev->event, pr);
>  
>  	tp->retprobe = (pr == 'r');
> @@ -1135,10 +1149,17 @@ static int parse_probe_trace_command(const char *cmd,
>  		p++;
>  	} else
>  		p = argv[1];
> -	ret = sscanf(p, "%a[^+]+%lu", (float *)(void *)&tp->symbol,
> -		     &tp->offset);
> -	if (ret == 1)
> +	fmt1_str = strtok_r(p, "+", &fmt);
> +	tp->symbol = strdup(fmt1_str);
> +	if (tp->symbol == NULL) {
> +		ret = -ENOMEM;
> +		goto out;
> +	}
> +	fmt2_str = strtok_r(NULL, "", &fmt);
> +	if (fmt2_str == NULL)
>  		tp->offset = 0;
> +	else
> +		tp->offset = strtoul(fmt2_str, NULL, 10);
>  
>  	tev->nargs = argc - 2;
>  	tev->args = zalloc(sizeof(struct probe_trace_arg) * tev->nargs);
> @@ -1162,6 +1183,7 @@ static int parse_probe_trace_command(const char *cmd,
>  	}
>  	ret = 0;
>  out:
> +	free(argv0_str);
>  	argv_free(argv);
>  	return ret;
>  }
> diff --git a/tools/perf/util/trace-event-parse.c b/tools/perf/util/trace-event-parse.c
> index aa4c860..3aabcd6 100644
> --- a/tools/perf/util/trace-event-parse.c
> +++ b/tools/perf/util/trace-event-parse.c
> @@ -229,24 +229,22 @@ void parse_proc_kallsyms(struct pevent *pevent,
>  	char *next = NULL;
>  	char *addr_str;
>  	char *mod;
> -	char ch;
> +	char *fmt;
>  
>  	line = strtok_r(file, "\n", &next);
>  	while (line) {
>  		mod = NULL;
> -		sscanf(line, "%as %c %as\t[%as",
> -		       (float *)(void *)&addr_str, /* workaround gcc warning */
> -		       &ch, (float *)(void *)&func, (float *)(void *)&mod);
> +		addr_str = strtok_r(line, " ", &fmt);
>  		addr = strtoull(addr_str, NULL, 16);
> -		free(addr_str);
> -
> -		/* truncate the extra ']' */
> +		/* skip character */
> +		strtok_r(NULL, " ", &fmt);
> +		func = strtok_r(NULL, "\t", &fmt);
> +		mod = strtok_r(NULL, "]", &fmt);
> +		/* truncate the extra '[' */
>  		if (mod)
> -			mod[strlen(mod) - 1] = 0;
> +			mod = mod + 1;
>  
>  		pevent_register_function(pevent, func, addr, mod);
> -		free(func);
> -		free(mod);
>  
>  		line = strtok_r(NULL, "\n", &next);
>  	}
> 


-- 
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com

[tip:perf/core] perf tools: remove sscanf extension %as

[tip-bot for Irina Tirdea]

Commit-ID:  bcbd004020bf0d725722be75da35fd326ff63ef4
Gitweb:     http://git.kernel.org/tip/bcbd004020bf0d725722be75da35fd326ff63ef4
Author:     Irina Tirdea <irina.tirdea@intel.com>
AuthorDate: Thu, 20 Sep 2012 23:37:50 +0300
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 24 Sep 2012 11:49:31 -0300

perf tools: remove sscanf extension %as

perf uses sscanf extension %as to read and allocate a string in the same
step.  This is a non-standard extension only present in new versions of
glibc.

Replacing the use of sscanf and %as with strtok_r calls in order to
parse a given string into its components.  This is needed in Android
since bionic does not support
%as extension for sscanf.

Reviewed-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Tested-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
Acked-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Namhyung Kim <namhyung.kim@lge.com>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Steven Rostedt <rostedt@goodmis.org>
Link: http://lkml.kernel.org/r/1348173470-4936-1-git-send-email-irina.tirdea@gmail.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/probe-event.c       |   36 ++++++++++++++++++++++++++++------
 tools/perf/util/trace-event-parse.c |   18 +++++++---------
 2 files changed, 37 insertions(+), 17 deletions(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 4ce04c2..49a256e 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -1100,6 +1100,7 @@ static int parse_probe_trace_command(const char *cmd,
 	struct probe_trace_point *tp = &tev->point;
 	char pr;
 	char *p;
+	char *argv0_str = NULL, *fmt, *fmt1_str, *fmt2_str, *fmt3_str;
 	int ret, i, argc;
 	char **argv;
 
@@ -1116,14 +1117,27 @@ static int parse_probe_trace_command(const char *cmd,
 	}
 
 	/* Scan event and group name. */
-	ret = sscanf(argv[0], "%c:%a[^/ \t]/%a[^ \t]",
-		     &pr, (float *)(void *)&tev->group,
-		     (float *)(void *)&tev->event);
-	if (ret != 3) {
+	argv0_str = strdup(argv[0]);
+	if (argv0_str == NULL) {
+		ret = -ENOMEM;
+		goto out;
+	}
+	fmt1_str = strtok_r(argv0_str, ":", &fmt);
+	fmt2_str = strtok_r(NULL, "/", &fmt);
+	fmt3_str = strtok_r(NULL, " \t", &fmt);
+	if (fmt1_str == NULL || strlen(fmt1_str) != 1 || fmt2_str == NULL
+	    || fmt3_str == NULL) {
 		semantic_error("Failed to parse event name: %s\n", argv[0]);
 		ret = -EINVAL;
 		goto out;
 	}
+	pr = fmt1_str[0];
+	tev->group = strdup(fmt2_str);
+	tev->event = strdup(fmt3_str);
+	if (tev->group == NULL || tev->event == NULL) {
+		ret = -ENOMEM;
+		goto out;
+	}
 	pr_debug("Group:%s Event:%s probe:%c\n", tev->group, tev->event, pr);
 
 	tp->retprobe = (pr == 'r');
@@ -1135,10 +1149,17 @@ static int parse_probe_trace_command(const char *cmd,
 		p++;
 	} else
 		p = argv[1];
-	ret = sscanf(p, "%a[^+]+%lu", (float *)(void *)&tp->symbol,
-		     &tp->offset);
-	if (ret == 1)
+	fmt1_str = strtok_r(p, "+", &fmt);
+	tp->symbol = strdup(fmt1_str);
+	if (tp->symbol == NULL) {
+		ret = -ENOMEM;
+		goto out;
+	}
+	fmt2_str = strtok_r(NULL, "", &fmt);
+	if (fmt2_str == NULL)
 		tp->offset = 0;
+	else
+		tp->offset = strtoul(fmt2_str, NULL, 10);
 
 	tev->nargs = argc - 2;
 	tev->args = zalloc(sizeof(struct probe_trace_arg) * tev->nargs);
@@ -1162,6 +1183,7 @@ static int parse_probe_trace_command(const char *cmd,
 	}
 	ret = 0;
 out:
+	free(argv0_str);
 	argv_free(argv);
 	return ret;
 }
diff --git a/tools/perf/util/trace-event-parse.c b/tools/perf/util/trace-event-parse.c
index aa4c860..3aabcd6 100644
--- a/tools/perf/util/trace-event-parse.c
+++ b/tools/perf/util/trace-event-parse.c
@@ -229,24 +229,22 @@ void parse_proc_kallsyms(struct pevent *pevent,
 	char *next = NULL;
 	char *addr_str;
 	char *mod;
-	char ch;
+	char *fmt;
 
 	line = strtok_r(file, "\n", &next);
 	while (line) {
 		mod = NULL;
-		sscanf(line, "%as %c %as\t[%as",
-		       (float *)(void *)&addr_str, /* workaround gcc warning */
-		       &ch, (float *)(void *)&func, (float *)(void *)&mod);
+		addr_str = strtok_r(line, " ", &fmt);
 		addr = strtoull(addr_str, NULL, 16);
-		free(addr_str);
-
-		/* truncate the extra ']' */
+		/* skip character */
+		strtok_r(NULL, " ", &fmt);
+		func = strtok_r(NULL, "\t", &fmt);
+		mod = strtok_r(NULL, "]", &fmt);
+		/* truncate the extra '[' */
 		if (mod)
-			mod[strlen(mod) - 1] = 0;
+			mod = mod + 1;
 
 		pevent_register_function(pevent, func, addr, mod);
-		free(func);
-		free(mod);
 
 		line = strtok_r(NULL, "\n", &next);
 	}