mtrace: Use a static buffer for printing [BZ #25947]
Checks
| Context |
Check |
Description |
| dj/TryBot-apply_patch |
success
|
Patch applied to master at the time it was sent
|
Commit Message
Use a static buffer for mtrace printing now that it no longer adds to
default libc footprint.
---
malloc/mtrace-impl.c | 14 +++-----------
1 file changed, 3 insertions(+), 11 deletions(-)
Comments
Ping!
On 7/22/21 6:54 PM, Siddhesh Poyarekar via Libc-alpha wrote:
> Use a static buffer for mtrace printing now that it no longer adds to
> default libc footprint.
> ---
> malloc/mtrace-impl.c | 14 +++-----------
> 1 file changed, 3 insertions(+), 11 deletions(-)
>
> diff --git a/malloc/mtrace-impl.c b/malloc/mtrace-impl.c
> index 0e10ab7f60..83008ca18f 100644
> --- a/malloc/mtrace-impl.c
> +++ b/malloc/mtrace-impl.c
> @@ -34,11 +34,8 @@
>
> #include <kernel-features.h>
>
> -#define TRACE_BUFFER_SIZE 512
> -
> static FILE *mallstream;
> static const char mallenv[] = "MALLOC_TRACE";
> -static char *malloc_trace_buffer;
>
> static void
> tr_where (const void *caller, Dl_info *info)
> @@ -184,16 +181,13 @@ do_mtrace (void)
> mallfile = secure_getenv (mallenv);
> if (mallfile != NULL)
> {
> - char *mtb = malloc (TRACE_BUFFER_SIZE);
> - if (mtb == NULL)
> - return;
> -
> mallstream = fopen (mallfile != NULL ? mallfile : "/dev/null", "wce");
> if (mallstream != NULL)
> {
> /* Be sure it doesn't malloc its buffer! */
> - malloc_trace_buffer = mtb;
> - setvbuf (mallstream, malloc_trace_buffer, _IOFBF, TRACE_BUFFER_SIZE);
> + static char tracebuf [512];
> +
> + setvbuf (mallstream, tracebuf, _IOFBF, sizeof (tracebuf));
> fprintf (mallstream, "= Start\n");
> if (!added_atexit_handler)
> {
> @@ -203,8 +197,6 @@ do_mtrace (void)
> }
> __malloc_debug_enable (MALLOC_MTRACE_HOOK);
> }
> - else
> - free (mtb);
> }
> }
>
>
Siddhesh Poyarekar via Libc-alpha <libc-alpha@sourceware.org> writes:
> -#define TRACE_BUFFER_SIZE 512
I think it would be prudent to keep this, instead of hard-coding the 512
in the code below. I don't feel strongly about it, since the 512 only
appears once.
> -static char *malloc_trace_buffer;
We could have kept this here, too, but I have no real preference about
it.
> - char *mtb = malloc (TRACE_BUFFER_SIZE);
> - if (mtb == NULL)
> - return;
Ok.
> /* Be sure it doesn't malloc its buffer! */
> - malloc_trace_buffer = mtb;
> - setvbuf (mallstream, malloc_trace_buffer, _IOFBF, TRACE_BUFFER_SIZE);
> + static char tracebuf [512];
> +
> + setvbuf (mallstream, tracebuf, _IOFBF, sizeof (tracebuf));
_IOFBF is same as before, so OK, but I wonder if _IOLBF (line buffered)
would be more user-friendly? I can't think of a use case for piping the
output through "tail -f" or something, and actually needing to wait for
it, but it's text output.
> - else
> - free (mtb);
Ok.
LGTM as-is, my comments are for pondering only ;-)
Reviewed-by: DJ Delorie <dj@redhat.com>
@@ -34,11 +34,8 @@
#include <kernel-features.h>
-#define TRACE_BUFFER_SIZE 512
-
static FILE *mallstream;
static const char mallenv[] = "MALLOC_TRACE";
-static char *malloc_trace_buffer;
static void
tr_where (const void *caller, Dl_info *info)
@@ -184,16 +181,13 @@ do_mtrace (void)
mallfile = secure_getenv (mallenv);
if (mallfile != NULL)
{
- char *mtb = malloc (TRACE_BUFFER_SIZE);
- if (mtb == NULL)
- return;
-
mallstream = fopen (mallfile != NULL ? mallfile : "/dev/null", "wce");
if (mallstream != NULL)
{
/* Be sure it doesn't malloc its buffer! */
- malloc_trace_buffer = mtb;
- setvbuf (mallstream, malloc_trace_buffer, _IOFBF, TRACE_BUFFER_SIZE);
+ static char tracebuf [512];
+
+ setvbuf (mallstream, tracebuf, _IOFBF, sizeof (tracebuf));
fprintf (mallstream, "= Start\n");
if (!added_atexit_handler)
{
@@ -203,8 +197,6 @@ do_mtrace (void)
}
__malloc_debug_enable (MALLOC_MTRACE_HOOK);
}
- else
- free (mtb);
}
}