* [lttng-dev] [PATCH] Avoid using the heap for small strings with tracef/tracelog
@ 2021-05-19 16:56 Norbert Lange via lttng-dev
2021-05-19 19:00 ` Mathieu Desnoyers via lttng-dev
0 siblings, 1 reply; 5+ messages in thread
From: Norbert Lange via lttng-dev @ 2021-05-19 16:56 UTC (permalink / raw)
To: lttng-dev
Try to use vsnprintf with a 512 Byte buffer on the Stack,
if that fails allocate a larger one.
Signed-off-by: Norbert Lange <nolange79@gmail.com>
---
liblttng-ust/tracef.c | 13 ++++++++++---
liblttng-ust/tracelog.c | 12 +++++++++---
2 files changed, 19 insertions(+), 6 deletions(-)
diff --git a/liblttng-ust/tracef.c b/liblttng-ust/tracef.c
index ea98e43e..18c29e25 100644
--- a/liblttng-ust/tracef.c
+++ b/liblttng-ust/tracef.c
@@ -32,17 +32,24 @@
void _lttng_ust_tracef(const char *fmt, ...)
{
va_list ap;
- char *msg;
+ char local_buf[512];
+ char *msg = local_buf;
int len;
va_start(ap, fmt);
- len = vasprintf(&msg, fmt, ap);
+ len = vsnprintf(local_buf, sizeof(local_buf), fmt, ap);
+ if (len >= sizeof(local_buf)) {
+ msg = (char *)malloc(len + 1);
+ len = msg ? vsnprintf(msg, len + 1, fmt, ap) : -1;
+ }
+
/* len does not include the final \0 */
if (len < 0)
goto end;
__tracepoint_cb_lttng_ust_tracef___event(msg, len,
LTTNG_UST_CALLER_IP());
- free(msg);
end:
+ if (msg != local_buf)
+ free(msg);
va_end(ap);
}
diff --git a/liblttng-ust/tracelog.c b/liblttng-ust/tracelog.c
index 65fc87ed..a5d110fa 100644
--- a/liblttng-ust/tracelog.c
+++ b/liblttng-ust/tracelog.c
@@ -35,19 +35,25 @@
const char *fmt, ...) \
{ \
va_list ap; \
- char *msg; \
+ char local_buf[512]; \
+ char *msg = local_buf; \
int len; \
\
va_start(ap, fmt); \
- len = vasprintf(&msg, fmt, ap); \
+ len = vsnprintf(local_buf, sizeof(local_buf), fmt, ap); \
+ if (len >= sizeof(local_buf)) { \
+ msg = (char *)malloc(len + 1); \
+ len = msg ? vsnprintf(msg, len + 1, fmt, ap) : -1; \
+ } \
/* len does not include the final \0 */ \
if (len < 0) \
goto end; \
__tracepoint_cb_lttng_ust_tracelog___##level(file, \
line, func, msg, len, \
LTTNG_UST_CALLER_IP()); \
- free(msg); \
end: \
+ if (msg != local_buf) \
+ free(msg); \
va_end(ap); \
}
--
2.30.2
_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [lttng-dev] [PATCH] Avoid using the heap for small strings with tracef/tracelog
2021-05-19 16:56 [lttng-dev] [PATCH] Avoid using the heap for small strings with tracef/tracelog Norbert Lange via lttng-dev
@ 2021-05-19 19:00 ` Mathieu Desnoyers via lttng-dev
2021-05-20 12:20 ` Norbert Lange via lttng-dev
0 siblings, 1 reply; 5+ messages in thread
From: Mathieu Desnoyers via lttng-dev @ 2021-05-19 19:00 UTC (permalink / raw)
To: Norbert Lange; +Cc: lttng-dev
----- On May 19, 2021, at 12:56 PM, lttng-dev lttng-dev@lists.lttng.org wrote:
> Try to use vsnprintf with a 512 Byte buffer on the Stack,
> if that fails allocate a larger one.
I agree with the approach.
Can we move the hardcoded "512" to a #define within src/common/tracer.h ?
Thanks,
Mathieu
>
> Signed-off-by: Norbert Lange <nolange79@gmail.com>
> ---
> liblttng-ust/tracef.c | 13 ++++++++++---
> liblttng-ust/tracelog.c | 12 +++++++++---
> 2 files changed, 19 insertions(+), 6 deletions(-)
>
> diff --git a/liblttng-ust/tracef.c b/liblttng-ust/tracef.c
> index ea98e43e..18c29e25 100644
> --- a/liblttng-ust/tracef.c
> +++ b/liblttng-ust/tracef.c
> @@ -32,17 +32,24 @@
> void _lttng_ust_tracef(const char *fmt, ...)
> {
> va_list ap;
> - char *msg;
> + char local_buf[512];
> + char *msg = local_buf;
> int len;
>
> va_start(ap, fmt);
> - len = vasprintf(&msg, fmt, ap);
> + len = vsnprintf(local_buf, sizeof(local_buf), fmt, ap);
> + if (len >= sizeof(local_buf)) {
> + msg = (char *)malloc(len + 1);
> + len = msg ? vsnprintf(msg, len + 1, fmt, ap) : -1;
> + }
> +
> /* len does not include the final \0 */
> if (len < 0)
> goto end;
> __tracepoint_cb_lttng_ust_tracef___event(msg, len,
> LTTNG_UST_CALLER_IP());
> - free(msg);
> end:
> + if (msg != local_buf)
> + free(msg);
> va_end(ap);
> }
> diff --git a/liblttng-ust/tracelog.c b/liblttng-ust/tracelog.c
> index 65fc87ed..a5d110fa 100644
> --- a/liblttng-ust/tracelog.c
> +++ b/liblttng-ust/tracelog.c
> @@ -35,19 +35,25 @@
> const char *fmt, ...) \
> { \
> va_list ap; \
> - char *msg; \
> + char local_buf[512]; \
> + char *msg = local_buf; \
> int len; \
> \
> va_start(ap, fmt); \
> - len = vasprintf(&msg, fmt, ap); \
> + len = vsnprintf(local_buf, sizeof(local_buf), fmt, ap); \
> + if (len >= sizeof(local_buf)) { \
> + msg = (char *)malloc(len + 1); \
> + len = msg ? vsnprintf(msg, len + 1, fmt, ap) : -1; \
> + } \
> /* len does not include the final \0 */ \
> if (len < 0) \
> goto end; \
> __tracepoint_cb_lttng_ust_tracelog___##level(file, \
> line, func, msg, len, \
> LTTNG_UST_CALLER_IP()); \
> - free(msg); \
> end: \
> + if (msg != local_buf) \
> + free(msg); \
> va_end(ap); \
> }
>
> --
> 2.30.2
>
> _______________________________________________
> lttng-dev mailing list
> lttng-dev@lists.lttng.org
> https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [lttng-dev] [PATCH] Avoid using the heap for small strings with tracef/tracelog
2021-05-19 19:00 ` Mathieu Desnoyers via lttng-dev
@ 2021-05-20 12:20 ` Norbert Lange via lttng-dev
2021-05-20 13:33 ` Mathieu Desnoyers via lttng-dev
0 siblings, 1 reply; 5+ messages in thread
From: Norbert Lange via lttng-dev @ 2021-05-20 12:20 UTC (permalink / raw)
To: Mathieu Desnoyers; +Cc: lttng-dev
Am Mi., 19. Mai 2021 um 21:00 Uhr schrieb Mathieu Desnoyers
<mathieu.desnoyers@efficios.com>:
>
> ----- On May 19, 2021, at 12:56 PM, lttng-dev lttng-dev@lists.lttng.org wrote:
>
> > Try to use vsnprintf with a 512 Byte buffer on the Stack,
> > if that fails allocate a larger one.
>
> I agree with the approach.
>
> Can we move the hardcoded "512" to a #define within src/common/tracer.h ?
>
> Thanks,
>
> Mathieu
>
> >
> > Signed-off-by: Norbert Lange <nolange79@gmail.com>
> > ---
> > liblttng-ust/tracef.c | 13 ++++++++++---
> > liblttng-ust/tracelog.c | 12 +++++++++---
> > 2 files changed, 19 insertions(+), 6 deletions(-)
> >
> > diff --git a/liblttng-ust/tracef.c b/liblttng-ust/tracef.c
> > index ea98e43e..18c29e25 100644
> > --- a/liblttng-ust/tracef.c
> > +++ b/liblttng-ust/tracef.c
> > @@ -32,17 +32,24 @@
> > void _lttng_ust_tracef(const char *fmt, ...)
> > {
> > va_list ap;
> > - char *msg;
> > + char local_buf[512];
> > + char *msg = local_buf;
> > int len;
> >
> > va_start(ap, fmt);
> > - len = vasprintf(&msg, fmt, ap);
> > + len = vsnprintf(local_buf, sizeof(local_buf), fmt, ap);
> > + if (len >= sizeof(local_buf)) {
> > + msg = (char *)malloc(len + 1);
> > + len = msg ? vsnprintf(msg, len + 1, fmt, ap) : -1;
> > + }
> > +
> > /* len does not include the final \0 */
> > if (len < 0)
> > goto end;
> > __tracepoint_cb_lttng_ust_tracef___event(msg, len,
> > LTTNG_UST_CALLER_IP());
> > - free(msg);
> > end:
> > + if (msg != local_buf)
> > + free(msg);
> > va_end(ap);
> > }
> > diff --git a/liblttng-ust/tracelog.c b/liblttng-ust/tracelog.c
> > index 65fc87ed..a5d110fa 100644
> > --- a/liblttng-ust/tracelog.c
> > +++ b/liblttng-ust/tracelog.c
> > @@ -35,19 +35,25 @@
> > const char *fmt, ...) \
> > { \
> > va_list ap; \
> > - char *msg; \
> > + char local_buf[512]; \
> > + char *msg = local_buf; \
> > int len; \
> > \
> > va_start(ap, fmt); \
> > - len = vasprintf(&msg, fmt, ap); \
> > + len = vsnprintf(local_buf, sizeof(local_buf), fmt, ap); \
> > + if (len >= sizeof(local_buf)) { \
> > + msg = (char *)malloc(len + 1); \
> > + len = msg ? vsnprintf(msg, len + 1, fmt, ap) : -1; \
> > + } \
> > /* len does not include the final \0 */ \
> > if (len < 0) \
> > goto end; \
> > __tracepoint_cb_lttng_ust_tracelog___##level(file, \
> > line, func, msg, len, \
> > LTTNG_UST_CALLER_IP()); \
> > - free(msg); \
> > end: \
> > + if (msg != local_buf) \
> > + free(msg); \
> > va_end(ap); \
> > }
> >
> > --
> > 2.30.2
> >
> > _______________________________________________
> > lttng-dev mailing list
> > lttng-dev@lists.lttng.org
> > https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
>
> --
> Mathieu Desnoyers
> EfficiOS Inc.
> http://www.efficios.com
This patch was intended for 2.12, no src/common/tracer.h there.
Also this patch is brocken, as a va_list cant be iterated twice.
_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [lttng-dev] [PATCH] Avoid using the heap for small strings with tracef/tracelog
2021-05-20 12:20 ` Norbert Lange via lttng-dev
@ 2021-05-20 13:33 ` Mathieu Desnoyers via lttng-dev
2021-05-20 13:44 ` Norbert Lange via lttng-dev
0 siblings, 1 reply; 5+ messages in thread
From: Mathieu Desnoyers via lttng-dev @ 2021-05-20 13:33 UTC (permalink / raw)
To: Norbert Lange; +Cc: lttng-dev
----- On May 20, 2021, at 8:20 AM, Norbert Lange nolange79@gmail.com wrote:
> This patch was intended for 2.12, no src/common/tracer.h there.
You should post feature patches against the master branch. Or if you just
want to send a RFC, and you work on an older branch, please state that the
patch is RFC and which branch the patch is against, but that work won't
be merged unless it's rebased against master.
> Also this patch is brocken, as a va_list cant be iterated twice.
Right. But you could easily do:
va_start() [ first use ] va_end()
then
va_start() [ second use ] va_end(), right ?
Thanks,
Mathieu
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [lttng-dev] [PATCH] Avoid using the heap for small strings with tracef/tracelog
2021-05-20 13:33 ` Mathieu Desnoyers via lttng-dev
@ 2021-05-20 13:44 ` Norbert Lange via lttng-dev
0 siblings, 0 replies; 5+ messages in thread
From: Norbert Lange via lttng-dev @ 2021-05-20 13:44 UTC (permalink / raw)
To: Mathieu Desnoyers; +Cc: lttng-dev
Am Do., 20. Mai 2021 um 15:33 Uhr schrieb Mathieu Desnoyers
<mathieu.desnoyers@efficios.com>:
>
> ----- On May 20, 2021, at 8:20 AM, Norbert Lange nolange79@gmail.com wrote:
>
> > This patch was intended for 2.12, no src/common/tracer.h there.
>
> You should post feature patches against the master branch. Or if you just
> want to send a RFC, and you work on an older branch, please state that the
> patch is RFC and which branch the patch is against, but that work won't
> be merged unless it's rebased against master.
Yeah, I read the contributing guide since then, sorry =)
> > Also this patch is brocken, as a va_list cant be iterated twice.
>
> Right. But you could easily do:
>
> va_start() [ first use ] va_end()
> then
> va_start() [ second use ] va_end(), right ?
New patch vs master does this.
Norbert
_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-05-20 13:44 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-19 16:56 [lttng-dev] [PATCH] Avoid using the heap for small strings with tracef/tracelog Norbert Lange via lttng-dev
2021-05-19 19:00 ` Mathieu Desnoyers via lttng-dev
2021-05-20 12:20 ` Norbert Lange via lttng-dev
2021-05-20 13:33 ` Mathieu Desnoyers via lttng-dev
2021-05-20 13:44 ` Norbert Lange via lttng-dev
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).