| Message ID | 20191123084130027.IPKB.12758.ppp.dion.ne.jp@dmta0005.auone-net.jp (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | trace-cmd: Improve /proc/kallsyms parsing | expand |
On Sat, 23 Nov 2019 17:41:29 +0900 Kusanagi Kouichi <slash@ac.auone-net.jp> wrote: > - Use the stricter format string. > - Avoid allocating memory. > - Check sscanf return value. > Hi Kusanagi, Thanks for the patch, small comment below. > Signed-off-by: Kusanagi Kouichi <slash@ac.auone-net.jp> > --- > lib/trace-cmd/trace-util.c | 29 +++++++++++++++-------------- > 1 file changed, 15 insertions(+), 14 deletions(-) > > diff --git a/lib/trace-cmd/trace-util.c b/lib/trace-cmd/trace-util.c > index 8aa3b6c..ef8e4f7 100644 > --- a/lib/trace-cmd/trace-util.c > +++ b/lib/trace-cmd/trace-util.c > @@ -80,39 +80,40 @@ void tracecmd_parse_proc_kallsyms(struct tep_handle *pevent, > char *func; > char *line; > char *next = NULL; > - char *addr_str; > char *mod; > char ch; > > line = strtok_r(file, "\n", &next); > while (line) { > + int n, func_start, func_end = 0, mod_start, mod_end = 0; Small nit. I usually prefer to not have so many variables defined on one line. Something more like this: int func_start, func_end = 0; int mod_start, mod_end = 0; int n; Just easier to review. The rest looks good. I'll play with it to see if there's any instances that it doesn't work. Thanks! -- Steve > + > mod = NULL; > errno = 0; > - sscanf(line, "%ms %c %ms\t[%ms", > - &addr_str, &ch, &func, &mod); > + n = sscanf(line, "%16llx %c %n%*s%n%*1[\t][%n%*s%n", > + &addr, &ch, &func_start, &func_end, &mod_start, &mod_end); > if (errno) { > - free(addr_str); > - free(func); > - free(mod); > perror("sscanf"); > return; > } > - addr = strtoull(addr_str, NULL, 16); > - free(addr_str); > > - /* truncate the extra ']' */ > - if (mod) > - mod[strlen(mod) - 1] = 0; > + if (n != 2 || !func_end) > + return; > > + func = line + func_start; > /* > * Hacks for > * - arm arch that adds a lot of bogus '$a' functions > * - x86-64 that reports per-cpu variable offsets as absolute */ > - if (func[0] != '$' && ch != 'A' && ch != 'a') > + if (func[0] != '$' && ch != 'A' && ch != 'a') { > + line[func_end] = 0; > + if (mod_end) { > + mod = line + mod_start; > + /* truncate the extra ']' */ > + line[mod_end - 1] = 0; > + } > tep_register_function(pevent, func, addr, mod); > - free(func); > - free(mod); > + } > > line = strtok_r(NULL, "\n", &next); > }
diff --git a/lib/trace-cmd/trace-util.c b/lib/trace-cmd/trace-util.c index 8aa3b6c..ef8e4f7 100644 --- a/lib/trace-cmd/trace-util.c +++ b/lib/trace-cmd/trace-util.c @@ -80,39 +80,40 @@ void tracecmd_parse_proc_kallsyms(struct tep_handle *pevent, char *func; char *line; char *next = NULL; - char *addr_str; char *mod; char ch; line = strtok_r(file, "\n", &next); while (line) { + int n, func_start, func_end = 0, mod_start, mod_end = 0; + mod = NULL; errno = 0; - sscanf(line, "%ms %c %ms\t[%ms", - &addr_str, &ch, &func, &mod); + n = sscanf(line, "%16llx %c %n%*s%n%*1[\t][%n%*s%n", + &addr, &ch, &func_start, &func_end, &mod_start, &mod_end); if (errno) { - free(addr_str); - free(func); - free(mod); perror("sscanf"); return; } - addr = strtoull(addr_str, NULL, 16); - free(addr_str); - /* truncate the extra ']' */ - if (mod) - mod[strlen(mod) - 1] = 0; + if (n != 2 || !func_end) + return; + func = line + func_start; /* * Hacks for * - arm arch that adds a lot of bogus '$a' functions * - x86-64 that reports per-cpu variable offsets as absolute */ - if (func[0] != '$' && ch != 'A' && ch != 'a') + if (func[0] != '$' && ch != 'A' && ch != 'a') { + line[func_end] = 0; + if (mod_end) { + mod = line + mod_start; + /* truncate the extra ']' */ + line[mod_end - 1] = 0; + } tep_register_function(pevent, func, addr, mod); - free(func); - free(mod); + } line = strtok_r(NULL, "\n", &next); }
- Use the stricter format string. - Avoid allocating memory. - Check sscanf return value. Signed-off-by: Kusanagi Kouichi <slash@ac.auone-net.jp> --- lib/trace-cmd/trace-util.c | 29 +++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-)