* [PATCH] perf probe: convert_name_to_addr() allocated the wrong size buffer for a function name
@ 2012-10-16 1:37 Hyeoncheol Lee
2012-10-16 4:19 ` Masami Hiramatsu
0 siblings, 1 reply; 5+ messages in thread
From: Hyeoncheol Lee @ 2012-10-16 1:37 UTC (permalink / raw)
To: acme; +Cc: LKML, Masami Hiramatsu, Srikar Dronamraju
convert_name_to_addr() allocated sizeof(char *) * MAX_PROBE_ARGS
bytes for a function name
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 | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 49a256e..bb40ed4 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -2352,13 +2352,14 @@ 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) *
+ (3 + sizeof(unsigned long long) * 2));
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);
+ sprintf(pp->function, "0x%llx", vaddr);
ret = 0;
out:
--
1.7.10.4
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] perf probe: convert_name_to_addr() allocated the wrong size buffer for a function name
2012-10-16 1:37 [PATCH] perf probe: convert_name_to_addr() allocated the wrong size buffer for a function name Hyeoncheol Lee
@ 2012-10-16 4:19 ` Masami Hiramatsu
2012-10-16 4:39 ` Srikar Dronamraju
2012-10-16 8:13 ` Hyeoncheol Lee
0 siblings, 2 replies; 5+ messages in thread
From: Masami Hiramatsu @ 2012-10-16 4:19 UTC (permalink / raw)
To: Hyeoncheol Lee
Cc: acme, LKML, Srikar Dronamraju, yrl.pp-manager.tt@hitachi.com
(2012/10/16 10:37), Hyeoncheol Lee wrote:
> convert_name_to_addr() allocated sizeof(char *) * MAX_PROBE_ARGS
> bytes for a function name
Yeah, that one was from my laziness...
>
> 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 | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> index 49a256e..bb40ed4 100644
> --- a/tools/perf/util/probe-event.c
> +++ b/tools/perf/util/probe-event.c
> @@ -2352,13 +2352,14 @@ 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) *
> + (3 + sizeof(unsigned long long) * 2));
Could you comment that this is enough long here?
> 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);
> + sprintf(pp->function, "0x%llx", vaddr);
And at least we should use snprintf instead of sprintf...
(I think ret = e_snprintf(...) is better)
> ret = 0;
>
> out:
>
Thank you,
--
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] 5+ messages in thread* Re: [PATCH] perf probe: convert_name_to_addr() allocated the wrong size buffer for a function name
2012-10-16 4:19 ` Masami Hiramatsu
@ 2012-10-16 4:39 ` Srikar Dronamraju
2012-10-16 8:16 ` Hyeoncheol Lee
2012-10-16 8:13 ` Hyeoncheol Lee
1 sibling, 1 reply; 5+ messages in thread
From: Srikar Dronamraju @ 2012-10-16 4:39 UTC (permalink / raw)
To: Masami Hiramatsu
Cc: Hyeoncheol Lee, acme, LKML, yrl.pp-manager.tt@hitachi.com
* Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> [2012-10-16 13:19:57]:
> (2012/10/16 10:37), Hyeoncheol Lee wrote:
> > convert_name_to_addr() allocated sizeof(char *) * MAX_PROBE_ARGS
> > bytes for a function name
>
> Yeah, that one was from my laziness...
>
Guess not your fault, but mine.
> >
> > 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 | 5 +++--
> > 1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> > index 49a256e..bb40ed4 100644
> > --- a/tools/perf/util/probe-event.c
> > +++ b/tools/perf/util/probe-event.c
> > @@ -2352,13 +2352,14 @@ 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) *
> > + (3 + sizeof(unsigned long long) * 2));
>
> Could you comment that this is enough long here?
Also can we move the arith into a macro?
>
> > 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);
> > + sprintf(pp->function, "0x%llx", vaddr);
>
> And at least we should use snprintf instead of sprintf...
> (I think ret = e_snprintf(...) is better)
>
Agree.
> > ret = 0;
> >
> > out:
> >
>
--
Thanks and Regards
Srikar
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] perf probe: convert_name_to_addr() allocated the wrong size buffer for a function name
2012-10-16 4:39 ` Srikar Dronamraju
@ 2012-10-16 8:16 ` Hyeoncheol Lee
0 siblings, 0 replies; 5+ messages in thread
From: Hyeoncheol Lee @ 2012-10-16 8:16 UTC (permalink / raw)
To: Srikar Dronamraju
Cc: Masami Hiramatsu, acme, LKML, yrl.pp-manager.tt@hitachi.com
Hi,
2012/10/16 Srikar Dronamraju <srikar@linux.vnet.ibm.com>:
> * Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> [2012-10-16 13:19:57]:
>
>> (2012/10/16 10:37), Hyeoncheol Lee wrote:
>> > convert_name_to_addr() allocated sizeof(char *) * MAX_PROBE_ARGS
>> > bytes for a function name
>>
>> Yeah, that one was from my laziness...
>>
>
> Guess not your fault, but mine.
>
>> >
>> > 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 | 5 +++--
>> > 1 file changed, 3 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
>> > index 49a256e..bb40ed4 100644
>> > --- a/tools/perf/util/probe-event.c
>> > +++ b/tools/perf/util/probe-event.c
>> > @@ -2352,13 +2352,14 @@ 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) *
>> > + (3 + sizeof(unsigned long long) * 2));
>>
>> Could you comment that this is enough long here?
>
> Also can we move the arith into a macro?
>
I will do.
>>
>> > 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);
>> > + sprintf(pp->function, "0x%llx", vaddr);
>>
>> And at least we should use snprintf instead of sprintf...
>> (I think ret = e_snprintf(...) is better)
>>
>
> Agree.
Yes
>
>> > ret = 0;
>> >
>> > out:
>> >
>>
>
> --
> Thanks and Regards
> Srikar
>
Thank you for your comment.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] perf probe: convert_name_to_addr() allocated the wrong size buffer for a function name
2012-10-16 4:19 ` Masami Hiramatsu
2012-10-16 4:39 ` Srikar Dronamraju
@ 2012-10-16 8:13 ` Hyeoncheol Lee
1 sibling, 0 replies; 5+ messages in thread
From: Hyeoncheol Lee @ 2012-10-16 8:13 UTC (permalink / raw)
To: Masami Hiramatsu
Cc: acme, LKML, Srikar Dronamraju, yrl.pp-manager.tt@hitachi.com
Hi,
2012/10/16 Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>:
> (2012/10/16 10:37), Hyeoncheol Lee wrote:
>> convert_name_to_addr() allocated sizeof(char *) * MAX_PROBE_ARGS
>> bytes for a function name
>
> Yeah, that one was from my laziness...
>
>>
>> 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 | 5 +++--
>> 1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
>> index 49a256e..bb40ed4 100644
>> --- a/tools/perf/util/probe-event.c
>> +++ b/tools/perf/util/probe-event.c
>> @@ -2352,13 +2352,14 @@ 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) *
>> + (3 + sizeof(unsigned long long) * 2));
>
> Could you comment that this is enough long here?
>
Because a hexadecimal address that starts with "0x"
will be stored in pp->function. sizeof(unsigned long long) * 2 is
maximum length of hexadecimal number of variable "vaddr"
and 3 bytes are for "0x" and null character.
>> 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);
>> + sprintf(pp->function, "0x%llx", vaddr);
>
> And at least we should use snprintf instead of sprintf...
> (I think ret = e_snprintf(...) is better)
>
You are right, but I didn't want to write down the length of
"pp->function" again.
>> ret = 0;
>>
>> out:
>>
>
> Thank you,
>
> --
> Masami HIRAMATSU
> IT Management Research Dept. Linux Technology Center
> Hitachi, Ltd., Yokohama Research Laboratory
> E-mail: masami.hiramatsu.pt@hitachi.com
>
>
Thank you very much!
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2012-10-16 8:16 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-10-16 1:37 [PATCH] perf probe: convert_name_to_addr() allocated the wrong size buffer for a function name Hyeoncheol Lee
2012-10-16 4:19 ` Masami Hiramatsu
2012-10-16 4:39 ` Srikar Dronamraju
2012-10-16 8:16 ` Hyeoncheol Lee
2012-10-16 8:13 ` Hyeoncheol Lee
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox