* [PATCH 0/4] perf tools: runtime fixes for Android
@ 2012-09-13 22:07 Irina Tirdea
2012-09-13 22:07 ` [PATCH 1/4] perf tools: remove sscanf extension %as Irina Tirdea
` (3 more replies)
0 siblings, 4 replies; 17+ messages in thread
From: Irina Tirdea @ 2012-09-13 22:07 UTC (permalink / raw)
To: mingo, acme, a.p.zijlstra, rostedt
Cc: paulus, dsahern, namhyung.kim, linux-kernel, 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
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 1/4] perf tools: remove sscanf extension %as
2012-09-13 22:07 [PATCH 0/4] perf tools: runtime fixes for Android Irina Tirdea
@ 2012-09-13 22:07 ` Irina Tirdea
2012-09-14 1:54 ` Namhyung Kim
2012-09-20 20:37 ` [PATCH v2 1/1] " irina.tirdea
2012-09-13 22:07 ` [PATCH 2/4] perf stat: add compile-time option to disable --big-num Irina Tirdea
` (2 subsequent siblings)
3 siblings, 2 replies; 17+ messages in thread
From: Irina Tirdea @ 2012-09-13 22:07 UTC (permalink / raw)
To: mingo, acme, a.p.zijlstra, rostedt
Cc: paulus, dsahern, namhyung.kim, linux-kernel, 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
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 2/4] perf stat: add compile-time option to disable --big-num
2012-09-13 22:07 [PATCH 0/4] perf tools: runtime fixes for Android Irina Tirdea
2012-09-13 22:07 ` [PATCH 1/4] perf tools: remove sscanf extension %as Irina Tirdea
@ 2012-09-13 22:07 ` Irina Tirdea
2012-09-14 5:40 ` Ingo Molnar
2012-09-23 21:48 ` [PATCH v2 1/1] perf stat: implement --big-num grouping Irina Tirdea
2012-09-13 22:07 ` [PATCH 3/4] perf archive: remove -f from the rm command Irina Tirdea
2012-09-13 22:07 ` [PATCH 4/4] perf archive: make f the last parameter for tar Irina Tirdea
3 siblings, 2 replies; 17+ messages in thread
From: Irina Tirdea @ 2012-09-13 22:07 UTC (permalink / raw)
To: mingo, acme, a.p.zijlstra, rostedt
Cc: paulus, dsahern, namhyung.kim, linux-kernel, 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
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 3/4] perf archive: remove -f from the rm command
2012-09-13 22:07 [PATCH 0/4] perf tools: runtime fixes for Android Irina Tirdea
2012-09-13 22:07 ` [PATCH 1/4] perf tools: remove sscanf extension %as Irina Tirdea
2012-09-13 22:07 ` [PATCH 2/4] perf stat: add compile-time option to disable --big-num Irina Tirdea
@ 2012-09-13 22:07 ` Irina Tirdea
2012-09-19 15:19 ` [tip:perf/core] perf archive: Remove " tip-bot for Irina Tirdea
2012-09-13 22:07 ` [PATCH 4/4] perf archive: make f the last parameter for tar Irina Tirdea
3 siblings, 1 reply; 17+ messages in thread
From: Irina Tirdea @ 2012-09-13 22:07 UTC (permalink / raw)
To: mingo, acme, a.p.zijlstra, rostedt
Cc: paulus, dsahern, namhyung.kim, linux-kernel, 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
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 4/4] perf archive: make f the last parameter for tar
2012-09-13 22:07 [PATCH 0/4] perf tools: runtime fixes for Android Irina Tirdea
` (2 preceding siblings ...)
2012-09-13 22:07 ` [PATCH 3/4] perf archive: remove -f from the rm command Irina Tirdea
@ 2012-09-13 22:07 ` Irina Tirdea
2012-09-19 15:20 ` [tip:perf/core] perf archive: Make 'f' " tip-bot for Irina Tirdea
3 siblings, 1 reply; 17+ messages in thread
From: Irina Tirdea @ 2012-09-13 22:07 UTC (permalink / raw)
To: mingo, acme, a.p.zijlstra, rostedt
Cc: paulus, dsahern, namhyung.kim, linux-kernel, 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
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 1/4] perf tools: remove sscanf extension %as
2012-09-13 22:07 ` [PATCH 1/4] perf tools: remove sscanf extension %as Irina Tirdea
@ 2012-09-14 1:54 ` Namhyung Kim
2012-09-19 3:20 ` Masami Hiramatsu
2012-09-20 20:37 ` [PATCH v2 1/1] " irina.tirdea
1 sibling, 1 reply; 17+ messages in thread
From: Namhyung Kim @ 2012-09-14 1:54 UTC (permalink / raw)
To: Irina Tirdea
Cc: mingo, acme, a.p.zijlstra, rostedt, paulus, dsahern,
namhyung.kim, linux-kernel, Irina Tirdea, masami.hiramatsu.pt
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;
> }
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/4] perf stat: add compile-time option to disable --big-num
2012-09-13 22:07 ` [PATCH 2/4] perf stat: add compile-time option to disable --big-num Irina Tirdea
@ 2012-09-14 5:40 ` Ingo Molnar
2012-09-20 19:17 ` Irina Tirdea
2012-09-23 21:48 ` [PATCH v2 1/1] perf stat: implement --big-num grouping Irina Tirdea
1 sibling, 1 reply; 17+ messages in thread
From: Ingo Molnar @ 2012-09-14 5:40 UTC (permalink / raw)
To: Irina Tirdea
Cc: mingo, acme, a.p.zijlstra, rostedt, paulus, dsahern,
namhyung.kim, linux-kernel, Irina Tirdea
* 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
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/4] perf tools: remove sscanf extension %as
2012-09-14 1:54 ` Namhyung Kim
@ 2012-09-19 3:20 ` Masami Hiramatsu
2012-09-20 19:13 ` Irina Tirdea
0 siblings, 1 reply; 17+ messages in thread
From: Masami Hiramatsu @ 2012-09-19 3:20 UTC (permalink / raw)
To: Namhyung Kim
Cc: Irina Tirdea, mingo, acme, a.p.zijlstra, rostedt, paulus,
dsahern, namhyung.kim, linux-kernel, Irina Tirdea
(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
^ permalink raw reply [flat|nested] 17+ messages in thread
* [tip:perf/core] perf archive: Remove -f from the rm command
2012-09-13 22:07 ` [PATCH 3/4] perf archive: remove -f from the rm command Irina Tirdea
@ 2012-09-19 15:19 ` tip-bot for Irina Tirdea
0 siblings, 0 replies; 17+ messages in thread
From: tip-bot for Irina Tirdea @ 2012-09-19 15:19 UTC (permalink / raw)
To: linux-tip-commits
Cc: acme, linux-kernel, paulus, mingo, hpa, mingo, a.p.zijlstra,
namhyung.kim, rostedt, irina.tirdea, dsahern, tglx
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."
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [tip:perf/core] perf archive: Make 'f' the last parameter for tar
2012-09-13 22:07 ` [PATCH 4/4] perf archive: make f the last parameter for tar Irina Tirdea
@ 2012-09-19 15:20 ` tip-bot for Irina Tirdea
0 siblings, 0 replies; 17+ messages in thread
From: tip-bot for Irina Tirdea @ 2012-09-19 15:20 UTC (permalink / raw)
To: linux-tip-commits
Cc: acme, linux-kernel, paulus, mingo, hpa, mingo, a.p.zijlstra,
namhyung.kim, rostedt, irina.tirdea, dsahern, tglx
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"
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 1/4] perf tools: remove sscanf extension %as
2012-09-19 3:20 ` Masami Hiramatsu
@ 2012-09-20 19:13 ` Irina Tirdea
0 siblings, 0 replies; 17+ messages in thread
From: Irina Tirdea @ 2012-09-20 19:13 UTC (permalink / raw)
To: Masami Hiramatsu, Namhyung Kim
Cc: mingo, acme, a.p.zijlstra, rostedt, paulus, dsahern,
namhyung.kim, linux-kernel, 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
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/4] perf stat: add compile-time option to disable --big-num
2012-09-14 5:40 ` Ingo Molnar
@ 2012-09-20 19:17 ` Irina Tirdea
0 siblings, 0 replies; 17+ messages in thread
From: Irina Tirdea @ 2012-09-20 19:17 UTC (permalink / raw)
To: Ingo Molnar
Cc: mingo, acme, a.p.zijlstra, rostedt, paulus, dsahern,
namhyung.kim, linux-kernel, 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
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v2 1/1] perf tools: remove sscanf extension %as
2012-09-13 22:07 ` [PATCH 1/4] perf tools: remove sscanf extension %as Irina Tirdea
2012-09-14 1:54 ` Namhyung Kim
@ 2012-09-20 20:37 ` irina.tirdea
2012-09-21 15:29 ` Arnaldo Carvalho de Melo
` (2 more replies)
1 sibling, 3 replies; 17+ messages in thread
From: irina.tirdea @ 2012-09-20 20:37 UTC (permalink / raw)
To: mingo, acme, a.p.zijlstra, rostedt
Cc: paulus, dsahern, namhyung.kim, linux-kernel, masami.hiramatsu.pt,
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
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH v2 1/1] perf tools: remove sscanf extension %as
2012-09-20 20:37 ` [PATCH v2 1/1] " irina.tirdea
@ 2012-09-21 15:29 ` Arnaldo Carvalho de Melo
2012-09-24 7:13 ` Masami Hiramatsu
2012-09-27 5:35 ` [tip:perf/core] " tip-bot for Irina Tirdea
2 siblings, 0 replies; 17+ messages in thread
From: Arnaldo Carvalho de Melo @ 2012-09-21 15:29 UTC (permalink / raw)
To: irina.tirdea
Cc: mingo, a.p.zijlstra, rostedt, paulus, dsahern, namhyung.kim,
linux-kernel, masami.hiramatsu.pt, Irina Tirdea
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
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v2 1/1] perf stat: implement --big-num grouping
2012-09-13 22:07 ` [PATCH 2/4] perf stat: add compile-time option to disable --big-num Irina Tirdea
2012-09-14 5:40 ` Ingo Molnar
@ 2012-09-23 21:48 ` Irina Tirdea
1 sibling, 0 replies; 17+ messages in thread
From: Irina Tirdea @ 2012-09-23 21:48 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Ingo Molnar, Steven Rostedt, Peter Zijlstra
Cc: LKML, Paul Mackerras, David Ahern, Namhyung Kim, Pekka Enberg,
Jiri Olsa, 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
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH v2 1/1] perf tools: remove sscanf extension %as
2012-09-20 20:37 ` [PATCH v2 1/1] " irina.tirdea
2012-09-21 15:29 ` Arnaldo Carvalho de Melo
@ 2012-09-24 7:13 ` Masami Hiramatsu
2012-09-27 5:35 ` [tip:perf/core] " tip-bot for Irina Tirdea
2 siblings, 0 replies; 17+ messages in thread
From: Masami Hiramatsu @ 2012-09-24 7:13 UTC (permalink / raw)
To: irina.tirdea
Cc: mingo, acme, a.p.zijlstra, rostedt, paulus, dsahern,
namhyung.kim, linux-kernel, Irina Tirdea, yrl.pp-manager.tt
(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
^ permalink raw reply [flat|nested] 17+ messages in thread
* [tip:perf/core] perf tools: remove sscanf extension %as
2012-09-20 20:37 ` [PATCH v2 1/1] " irina.tirdea
2012-09-21 15:29 ` Arnaldo Carvalho de Melo
2012-09-24 7:13 ` Masami Hiramatsu
@ 2012-09-27 5:35 ` tip-bot for Irina Tirdea
2 siblings, 0 replies; 17+ messages in thread
From: tip-bot for Irina Tirdea @ 2012-09-27 5:35 UTC (permalink / raw)
To: linux-tip-commits
Cc: acme, linux-kernel, paulus, mingo, hpa, mingo, a.p.zijlstra,
namhyung.kim, masami.hiramatsu.pt, rostedt, irina.tirdea,
dsahern, tglx
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);
}
^ permalink raw reply related [flat|nested] 17+ messages in thread
end of thread, other threads:[~2012-09-27 5:36 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-09-13 22:07 [PATCH 0/4] perf tools: runtime fixes for Android Irina Tirdea
2012-09-13 22:07 ` [PATCH 1/4] perf tools: remove sscanf extension %as Irina Tirdea
2012-09-14 1:54 ` Namhyung Kim
2012-09-19 3:20 ` Masami Hiramatsu
2012-09-20 19:13 ` Irina Tirdea
2012-09-20 20:37 ` [PATCH v2 1/1] " irina.tirdea
2012-09-21 15:29 ` Arnaldo Carvalho de Melo
2012-09-24 7:13 ` Masami Hiramatsu
2012-09-27 5:35 ` [tip:perf/core] " tip-bot for Irina Tirdea
2012-09-13 22:07 ` [PATCH 2/4] perf stat: add compile-time option to disable --big-num Irina Tirdea
2012-09-14 5:40 ` Ingo Molnar
2012-09-20 19:17 ` Irina Tirdea
2012-09-23 21:48 ` [PATCH v2 1/1] perf stat: implement --big-num grouping Irina Tirdea
2012-09-13 22:07 ` [PATCH 3/4] perf archive: remove -f from the rm command Irina Tirdea
2012-09-19 15:19 ` [tip:perf/core] perf archive: Remove " tip-bot for Irina Tirdea
2012-09-13 22:07 ` [PATCH 4/4] perf archive: make f the last parameter for tar Irina Tirdea
2012-09-19 15:20 ` [tip:perf/core] perf archive: Make 'f' " tip-bot for Irina Tirdea
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).