(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.tir...@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.tir...@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...@hitachi.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
