Message ID | 20200420134920.93473-1-chenzefeng2@huawei.com |
---|---|
State | New |
Headers | show |
Series | [v2] malloc: fix memleak in function muntrace | expand |
* chenzefeng: > when we call functons as follow: > mtrace(); > ... > muntrace(); > It would cause memleak, for the mtrace malloc some memory: > mtd = malloc(TRACE_BUFFER_SIZE); > and it would not be free. Therefor it should be freed in muntrace. > > Signed-off-by: chenzefeng <chenzefeng2@huawei.com> glibc uses copyright assignment to the FSF for contributions, and not the DCO. I trust this contribution is covered by Huawei's copyright assignment? > --- > malloc/mtrace.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/malloc/mtrace.c b/malloc/mtrace.c > index 7e7719df97..ab65ebe764 100644 > --- a/malloc/mtrace.c > +++ b/malloc/mtrace.c > @@ -365,4 +365,6 @@ muntrace (void) > > fprintf (f, "= End\n"); > fclose (f); > + if (malloc_trace_buffer != NULL) > + free (malloc_trace_buffer); > } I think you can call free unconditionally.
On Mon, Apr 20, 2020 at 10:50 AM Florian Weimer <fw@deneb.enyo.de> wrote: > * chenzefeng: > > + if (malloc_trace_buffer != NULL) > > + free (malloc_trace_buffer); > > } > > I think you can call free unconditionally. I was going to say the same thing. (free(NULL) is specified to do nothing.) Also, it would be good to set malloc_trace_buffer to NULL after calling free. Also, __release_libc_mem should be calling muntrace (after calling __libc_freeres). zw
* Zack Weinberg: > On Mon, Apr 20, 2020 at 10:50 AM Florian Weimer <fw@deneb.enyo.de> wrote: >> * chenzefeng: >> > + if (malloc_trace_buffer != NULL) >> > + free (malloc_trace_buffer); >> > } >> >> I think you can call free unconditionally. > > I was going to say the same thing. (free(NULL) is specified to do nothing.) > > Also, it would be good to set malloc_trace_buffer to NULL after > calling free. This introduces an asymmetry with the handling of f. The existing code is fine either way.
On Mon, Apr 20, 2020 at 12:19 PM Florian Weimer <fw@deneb.enyo.de> wrote: > * Zack Weinberg: > > On Mon, Apr 20, 2020 at 10:50 AM Florian Weimer <fw@deneb.enyo.de> wrote: > >> * chenzefeng: > >> > + if (malloc_trace_buffer != NULL) > >> > + free (malloc_trace_buffer); > >> > } > >> > >> I think you can call free unconditionally. > > > > I was going to say the same thing. (free(NULL) is specified to do nothing.) > > > > Also, it would be good to set malloc_trace_buffer to NULL after > > calling free. > > This introduces an asymmetry with the handling of f. f is a local variable. muntrace does set mallstream to NULL. zw
* Zack Weinberg: > On Mon, Apr 20, 2020 at 12:19 PM Florian Weimer <fw@deneb.enyo.de> wrote: >> * Zack Weinberg: >> > On Mon, Apr 20, 2020 at 10:50 AM Florian Weimer <fw@deneb.enyo.de> wrote: >> >> * chenzefeng: >> >> > + if (malloc_trace_buffer != NULL) >> >> > + free (malloc_trace_buffer); >> >> > } >> >> >> >> I think you can call free unconditionally. >> > >> > I was going to say the same thing. (free(NULL) is specified to do nothing.) >> > >> > Also, it would be good to set malloc_trace_buffer to NULL after >> > calling free. >> >> This introduces an asymmetry with the handling of f. > > f is a local variable. muntrace does set mallstream to NULL. Oh in this case, malloc_trace_buffer should be reset as well.
diff --git a/malloc/mtrace.c b/malloc/mtrace.c index 7e7719df97..ab65ebe764 100644 --- a/malloc/mtrace.c +++ b/malloc/mtrace.c @@ -365,4 +365,6 @@ muntrace (void) fprintf (f, "= End\n"); fclose (f); + if (malloc_trace_buffer != NULL) + free (malloc_trace_buffer); }
when we call functons as follow: mtrace(); ... muntrace(); It would cause memleak, for the mtrace malloc some memory: mtd = malloc(TRACE_BUFFER_SIZE); and it would not be free. Therefor it should be freed in muntrace. Signed-off-by: chenzefeng <chenzefeng2@huawei.com> --- malloc/mtrace.c | 2 ++ 1 file changed, 2 insertions(+)