* [PATCH] perf probe: convert_name_to_addr() allocated the wrong size buffers for names @ 2012-10-17 8:28 Hyeoncheol Lee 2012-10-17 11:06 ` Masami Hiramatsu 0 siblings, 1 reply; 4+ messages in thread From: Hyeoncheol Lee @ 2012-10-17 8:28 UTC (permalink / raw) To: acme; +Cc: LKML, Masami Hiramatsu, Srikar Dronamraju The function allocated wrong size buffers for names Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> Cc: Srikar Dronamraju <srikar@linux.vnet.ibm.com> Signed-off-by: Hyeoncheol Lee <hyc.lee@gmail.com> --- tools/perf/util/probe-event.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c index 49a256e..5cd7c42 100644 --- a/tools/perf/util/probe-event.c +++ b/tools/perf/util/probe-event.c @@ -2273,6 +2273,8 @@ int show_available_funcs(const char *target, struct strfilter *_filter, return available_user_funcs(target); } +#define MAX_ADDRNAME_LEN (3 + sizeof(unsigned long long) * 2) + /* * uprobe_events only accepts address: * Convert function and any offset to address @@ -2332,7 +2334,7 @@ static int convert_name_to_addr(struct perf_probe_event *pev, const char *exec) if (!pev->group) { char *ptr1, *ptr2, *exec_copy; - pev->group = zalloc(sizeof(char *) * 64); + pev->group = zalloc(sizeof(char) * 64); exec_copy = strdup(exec); if (!exec_copy) { ret = -ENOMEM; @@ -2352,14 +2354,15 @@ static int convert_name_to_addr(struct perf_probe_event *pev, const char *exec) free(exec_copy); } free(pp->function); - pp->function = zalloc(sizeof(char *) * MAX_PROBE_ARGS); + pp->function = zalloc(sizeof(char) * MAX_ADDRNAME_LEN); if (!pp->function) { ret = -ENOMEM; pr_warning("Failed to allocate memory by zalloc.\n"); goto out; } - e_snprintf(pp->function, MAX_PROBE_ARGS, "0x%llx", vaddr); - ret = 0; + ret = e_snprintf(pp->function, MAX_ADDRNAME_LEN, "0x%llx", vaddr); + if (ret > 0) + ret = 0; out: if (map) { -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] perf probe: convert_name_to_addr() allocated the wrong size buffers for names 2012-10-17 8:28 [PATCH] perf probe: convert_name_to_addr() allocated the wrong size buffers for names Hyeoncheol Lee @ 2012-10-17 11:06 ` Masami Hiramatsu 2012-10-18 1:20 ` Namhyung Kim 0 siblings, 1 reply; 4+ messages in thread From: Masami Hiramatsu @ 2012-10-17 11:06 UTC (permalink / raw) To: Hyeoncheol Lee Cc: acme, LKML, Srikar Dronamraju, yrl.pp-manager.tt@hitachi.com (2012/10/17 17:28), Hyeoncheol Lee wrote: > The function allocated wrong size buffers for names > > Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> > Cc: Srikar Dronamraju <srikar@linux.vnet.ibm.com> > Signed-off-by: Hyeoncheol Lee <hyc.lee@gmail.com> > --- > tools/perf/util/probe-event.c | 11 +++++++---- > 1 file changed, 7 insertions(+), 4 deletions(-) > > diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c > index 49a256e..5cd7c42 100644 > --- a/tools/perf/util/probe-event.c > +++ b/tools/perf/util/probe-event.c > @@ -2273,6 +2273,8 @@ int show_available_funcs(const char *target, struct strfilter *_filter, > return available_user_funcs(target); > } > Please add a comment like below on the code. /* * sizeof(type) * 2 returns the maximum length of hexadecimal number * of type, because each half byte is encoding in one hex character. * Another 3 bytes are for "0x" and null character. */ Others are OK for me :) Thanks! > +#define MAX_ADDRNAME_LEN (3 + sizeof(unsigned long long) * 2) > + > /* > * uprobe_events only accepts address: > * Convert function and any offset to address > @@ -2332,7 +2334,7 @@ static int convert_name_to_addr(struct perf_probe_event *pev, const char *exec) > if (!pev->group) { > char *ptr1, *ptr2, *exec_copy; > > - pev->group = zalloc(sizeof(char *) * 64); > + pev->group = zalloc(sizeof(char) * 64); > exec_copy = strdup(exec); > if (!exec_copy) { > ret = -ENOMEM; > @@ -2352,14 +2354,15 @@ static int convert_name_to_addr(struct perf_probe_event *pev, const char *exec) > free(exec_copy); > } > free(pp->function); > - pp->function = zalloc(sizeof(char *) * MAX_PROBE_ARGS); > + pp->function = zalloc(sizeof(char) * MAX_ADDRNAME_LEN); > if (!pp->function) { > ret = -ENOMEM; > pr_warning("Failed to allocate memory by zalloc.\n"); > goto out; > } > - e_snprintf(pp->function, MAX_PROBE_ARGS, "0x%llx", vaddr); > - ret = 0; > + ret = e_snprintf(pp->function, MAX_ADDRNAME_LEN, "0x%llx", vaddr); > + if (ret > 0) > + ret = 0; > > out: > if (map) { > -- Masami HIRAMATSU IT Management Research Dept. Linux Technology Center Hitachi, Ltd., Yokohama Research Laboratory E-mail: masami.hiramatsu.pt@hitachi.com ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] perf probe: convert_name_to_addr() allocated the wrong size buffers for names 2012-10-17 11:06 ` Masami Hiramatsu @ 2012-10-18 1:20 ` Namhyung Kim 2012-10-18 3:10 ` Masami Hiramatsu 0 siblings, 1 reply; 4+ messages in thread From: Namhyung Kim @ 2012-10-18 1:20 UTC (permalink / raw) To: Masami Hiramatsu Cc: Hyeoncheol Lee, acme, LKML, Srikar Dronamraju, yrl.pp-manager.tt@hitachi.com Hi, On Wed, 17 Oct 2012 20:06:54 +0900, Masami Hiramatsu wrote: > (2012/10/17 17:28), Hyeoncheol Lee wrote: >> The function allocated wrong size buffers for names >> >> Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> >> Cc: Srikar Dronamraju <srikar@linux.vnet.ibm.com> >> Signed-off-by: Hyeoncheol Lee <hyc.lee@gmail.com> >> --- >> tools/perf/util/probe-event.c | 11 +++++++---- >> 1 file changed, 7 insertions(+), 4 deletions(-) >> >> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c >> index 49a256e..5cd7c42 100644 >> --- a/tools/perf/util/probe-event.c >> +++ b/tools/perf/util/probe-event.c >> @@ -2273,6 +2273,8 @@ int show_available_funcs(const char *target, struct strfilter *_filter, >> return available_user_funcs(target); >> } >> > > Please add a comment like below on the code. > > /* > * sizeof(type) * 2 returns the maximum length of hexadecimal number > * of type, because each half byte is encoding in one hex character. > * Another 3 bytes are for "0x" and null character. > */ > > Others are OK for me :) > > Thanks! > >> +#define MAX_ADDRNAME_LEN (3 + sizeof(unsigned long long) * 2) >> + >> /* >> * uprobe_events only accepts address: >> * Convert function and any offset to address >> @@ -2332,7 +2334,7 @@ static int convert_name_to_addr(struct perf_probe_event *pev, const char *exec) >> if (!pev->group) { >> char *ptr1, *ptr2, *exec_copy; >> >> - pev->group = zalloc(sizeof(char *) * 64); >> + pev->group = zalloc(sizeof(char) * 64); I guess we don't need this 'sizeof(char)' part? >> exec_copy = strdup(exec); >> if (!exec_copy) { >> ret = -ENOMEM; >> @@ -2352,14 +2354,15 @@ static int convert_name_to_addr(struct perf_probe_event *pev, const char *exec) >> free(exec_copy); >> } >> free(pp->function); >> - pp->function = zalloc(sizeof(char *) * MAX_PROBE_ARGS); >> + pp->function = zalloc(sizeof(char) * MAX_ADDRNAME_LEN); Ditto. Thanks, Namhyung >> if (!pp->function) { >> ret = -ENOMEM; >> pr_warning("Failed to allocate memory by zalloc.\n"); >> goto out; >> } >> - e_snprintf(pp->function, MAX_PROBE_ARGS, "0x%llx", vaddr); >> - ret = 0; >> + ret = e_snprintf(pp->function, MAX_ADDRNAME_LEN, "0x%llx", vaddr); >> + if (ret > 0) >> + ret = 0; >> >> out: >> if (map) { >> ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] perf probe: convert_name_to_addr() allocated the wrong size buffers for names 2012-10-18 1:20 ` Namhyung Kim @ 2012-10-18 3:10 ` Masami Hiramatsu 0 siblings, 0 replies; 4+ messages in thread From: Masami Hiramatsu @ 2012-10-18 3:10 UTC (permalink / raw) To: Namhyung Kim Cc: Hyeoncheol Lee, acme, LKML, Srikar Dronamraju, yrl.pp-manager.tt@hitachi.com (2012/10/18 10:20), Namhyung Kim wrote: > Hi, > > On Wed, 17 Oct 2012 20:06:54 +0900, Masami Hiramatsu wrote: >> (2012/10/17 17:28), Hyeoncheol Lee wrote: >>> The function allocated wrong size buffers for names >>> >>> Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> >>> Cc: Srikar Dronamraju <srikar@linux.vnet.ibm.com> >>> Signed-off-by: Hyeoncheol Lee <hyc.lee@gmail.com> >>> --- >>> tools/perf/util/probe-event.c | 11 +++++++---- >>> 1 file changed, 7 insertions(+), 4 deletions(-) >>> >>> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c >>> index 49a256e..5cd7c42 100644 >>> --- a/tools/perf/util/probe-event.c >>> +++ b/tools/perf/util/probe-event.c >>> @@ -2273,6 +2273,8 @@ int show_available_funcs(const char *target, struct strfilter *_filter, >>> return available_user_funcs(target); >>> } >>> >> >> Please add a comment like below on the code. >> >> /* >> * sizeof(type) * 2 returns the maximum length of hexadecimal number >> * of type, because each half byte is encoding in one hex character. >> * Another 3 bytes are for "0x" and null character. >> */ >> >> Others are OK for me :) >> >> Thanks! >> >>> +#define MAX_ADDRNAME_LEN (3 + sizeof(unsigned long long) * 2) >>> + >>> /* >>> * uprobe_events only accepts address: >>> * Convert function and any offset to address >>> @@ -2332,7 +2334,7 @@ static int convert_name_to_addr(struct perf_probe_event *pev, const char *exec) >>> if (!pev->group) { >>> char *ptr1, *ptr2, *exec_copy; >>> >>> - pev->group = zalloc(sizeof(char *) * 64); >>> + pev->group = zalloc(sizeof(char) * 64); > > I guess we don't need this 'sizeof(char)' part? Indeed, we don't need it from the viewpoint of C. However I'd like to keep that, because it gives us a hint that this buffer is used for string. Thank you, > > >>> exec_copy = strdup(exec); >>> if (!exec_copy) { >>> ret = -ENOMEM; >>> @@ -2352,14 +2354,15 @@ static int convert_name_to_addr(struct perf_probe_event *pev, const char *exec) >>> free(exec_copy); >>> } >>> free(pp->function); >>> - pp->function = zalloc(sizeof(char *) * MAX_PROBE_ARGS); >>> + pp->function = zalloc(sizeof(char) * MAX_ADDRNAME_LEN); > > Ditto. > > Thanks, > Namhyung > > >>> if (!pp->function) { >>> ret = -ENOMEM; >>> pr_warning("Failed to allocate memory by zalloc.\n"); >>> goto out; >>> } >>> - e_snprintf(pp->function, MAX_PROBE_ARGS, "0x%llx", vaddr); >>> - ret = 0; >>> + ret = e_snprintf(pp->function, MAX_ADDRNAME_LEN, "0x%llx", vaddr); >>> + if (ret > 0) >>> + ret = 0; >>> >>> out: >>> if (map) { >>> -- Masami HIRAMATSU IT Management Research Dept. Linux Technology Center Hitachi, Ltd., Yokohama Research Laboratory E-mail: masami.hiramatsu.pt@hitachi.com ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2012-10-18 3:10 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-10-17 8:28 [PATCH] perf probe: convert_name_to_addr() allocated the wrong size buffers for names Hyeoncheol Lee 2012-10-17 11:06 ` Masami Hiramatsu 2012-10-18 1:20 ` Namhyung Kim 2012-10-18 3:10 ` Masami Hiramatsu
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox