| Message ID | ed35e9e72aa77c9376e9c8a8f3a5443703fe6fbe.1566168923.git.DirtY.iCE.hu@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Multiple simultaneous audio backends | expand |
Hi Zoltán, On 8/19/19 1:06 AM, Kővágó, Zoltán wrote: > Signed-off-by: Kővágó, Zoltán <DirtY.iCE.hu@gmail.com> > --- > audio/audio.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/audio/audio.c b/audio/audio.c > index 924dddf2e7..9b28abca14 100644 > --- a/audio/audio.c > +++ b/audio/audio.c > @@ -1343,6 +1343,12 @@ static void free_audio_state(AudioState *s) > qapi_free_Audiodev(s->dev); > s->dev = NULL; > } > + > + if (s->ts) { > + timer_free(s->ts); > + s->ts = NULL; > + } Why not directly fix audio_cleanup() previous to your series? > + > g_free(s); > } > >
Hi, On 2019-08-19 01:25, Philippe Mathieu-Daudé wrote: > Hi Zoltán, > > On 8/19/19 1:06 AM, Kővágó, Zoltán wrote: >> Signed-off-by: Kővágó, Zoltán <DirtY.iCE.hu@gmail.com> >> --- >> audio/audio.c | 6 ++++++ >> 1 file changed, 6 insertions(+) >> >> diff --git a/audio/audio.c b/audio/audio.c >> index 924dddf2e7..9b28abca14 100644 >> --- a/audio/audio.c >> +++ b/audio/audio.c >> @@ -1343,6 +1343,12 @@ static void free_audio_state(AudioState *s) >> qapi_free_Audiodev(s->dev); >> s->dev = NULL; >> } >> + >> + if (s->ts) { >> + timer_free(s->ts); >> + s->ts = NULL; >> + } > > Why not directly fix audio_cleanup() previous to your series? I didn't really think about it. When I found the memory leak and figured out it wasn't made by one of my patches, I just patched it on top of my worktree. > >> + >> g_free(s); >> } >> >> Regards, Zoltan
On 8/19/19 4:05 AM, Zoltán Kővágó wrote: > Hi, > > On 2019-08-19 01:25, Philippe Mathieu-Daudé wrote: >> Hi Zoltán, >> >> On 8/19/19 1:06 AM, Kővágó, Zoltán wrote: >>> Signed-off-by: Kővágó, Zoltán <DirtY.iCE.hu@gmail.com> >>> --- >>> audio/audio.c | 6 ++++++ >>> 1 file changed, 6 insertions(+) >>> >>> diff --git a/audio/audio.c b/audio/audio.c >>> index 924dddf2e7..9b28abca14 100644 >>> --- a/audio/audio.c >>> +++ b/audio/audio.c >>> @@ -1343,6 +1343,12 @@ static void free_audio_state(AudioState *s) >>> qapi_free_Audiodev(s->dev); >>> s->dev = NULL; >>> } >>> + >>> + if (s->ts) { >>> + timer_free(s->ts); >>> + s->ts = NULL; >>> + } >> >> Why not directly fix audio_cleanup() previous to your series? > > I didn't really think about it. When I found the memory leak and > figured out it wasn't made by one of my patches, I just patched it on > top of my worktree. I see. QEMU does 'stable' releases from time to time. These releases contain security/bug fixes, and no new features. Sometimes important memory leaks are fixed in stable release. If this patch is at the beginning of your series, it might be selected for stable release. If it is after you introduced your feature it won't. If you have to respin your series, consider reordering this fix for stable inclusion. Meanwhile, Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> >> >>> + >>> g_free(s); >>> } >>> >>> > > Regards, > Zoltan >
diff --git a/audio/audio.c b/audio/audio.c index 924dddf2e7..9b28abca14 100644 --- a/audio/audio.c +++ b/audio/audio.c @@ -1343,6 +1343,12 @@ static void free_audio_state(AudioState *s) qapi_free_Audiodev(s->dev); s->dev = NULL; } + + if (s->ts) { + timer_free(s->ts); + s->ts = NULL; + } + g_free(s); }
Signed-off-by: Kővágó, Zoltán <DirtY.iCE.hu@gmail.com> --- audio/audio.c | 6 ++++++ 1 file changed, 6 insertions(+)