Re: [Qemu-block] [Qemu-devel] [PATCH 2/4] tests: fix bdrv-drain leak
Max Reitz writes: > On 2018-08-13 08:15, Markus Armbruster wrote: >> Max Reitz writes: >> >>> On 2018-08-10 08:28, Markus Armbruster wrote: Marc-André Lureau writes: > Spotted by ASAN: > > = > ==5378==ERROR: LeakSanitizer: detected memory leaks > > Direct leak of 65536 byte(s) in 1 object(s) allocated from: > #0 0x7f788f83bc48 in malloc (/lib64/libasan.so.5+0xeec48) > #1 0x7f788c9923c5 in g_malloc (/lib64/libglib-2.0.so.0+0x523c5) > #2 0x5622a1fe37bc in coroutine_trampoline > /home/elmarco/src/qq/util/coroutine-ucontext.c:116 > #3 0x7f788a15d75f in __correctly_grouped_prefixwc > (/lib64/libc.so.6+0x4c75f) > > Signed-off-by: Marc-André Lureau > --- > tests/test-bdrv-drain.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/tests/test-bdrv-drain.c b/tests/test-bdrv-drain.c > index 17bb8508ae..abc8bbe6f0 100644 > --- a/tests/test-bdrv-drain.c > +++ b/tests/test-bdrv-drain.c > @@ -948,6 +948,7 @@ static void coroutine_fn test_co_delete_by_drain(void > *opaque) > } > > dbdd->done = true; > +g_free(buffer); > } > > /** Screwed up in commit 4c8158e359d. Max, could the coroutine's stack accomodate the buffer? >>> >>> I don't know. How big is a coroutine's stack? The buffer size >>> apparently is 64k. (Which is my opinion is generally too much to put on >>> any stack.) >> >> Even in tests? > > You have a point, but why do things differently in tests for no reason? > > (So I don't leak memory, yeah, sure, but why not write the test in Rust > then from the start? O:-P) Only 230 days to April 1st, better hurry. >>> But why would you want to put it on the stack anyway? It's not like it >>> would make the patch any simpler. >> >> If you like the patch as is, go merge it as is :) > > W-well, I wasn't CC-d! I fixed that :) > I take this as a request to take this one patch from the series and > merge it into my tree, so I will do that. O:-) Thanks!
Re: [Qemu-block] [Qemu-devel] [PATCH 2/4] tests: fix bdrv-drain leak
On 2018-08-13 08:15, Markus Armbruster wrote: > Max Reitz writes: > >> On 2018-08-10 08:28, Markus Armbruster wrote: >>> Marc-André Lureau writes: >>> Spotted by ASAN: = ==5378==ERROR: LeakSanitizer: detected memory leaks Direct leak of 65536 byte(s) in 1 object(s) allocated from: #0 0x7f788f83bc48 in malloc (/lib64/libasan.so.5+0xeec48) #1 0x7f788c9923c5 in g_malloc (/lib64/libglib-2.0.so.0+0x523c5) #2 0x5622a1fe37bc in coroutine_trampoline /home/elmarco/src/qq/util/coroutine-ucontext.c:116 #3 0x7f788a15d75f in __correctly_grouped_prefixwc (/lib64/libc.so.6+0x4c75f) Signed-off-by: Marc-André Lureau --- tests/test-bdrv-drain.c | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/test-bdrv-drain.c b/tests/test-bdrv-drain.c index 17bb8508ae..abc8bbe6f0 100644 --- a/tests/test-bdrv-drain.c +++ b/tests/test-bdrv-drain.c @@ -948,6 +948,7 @@ static void coroutine_fn test_co_delete_by_drain(void *opaque) } dbdd->done = true; +g_free(buffer); } /** >>> >>> Screwed up in commit 4c8158e359d. >>> >>> Max, could the coroutine's stack accomodate the buffer? >> >> I don't know. How big is a coroutine's stack? The buffer size >> apparently is 64k. (Which is my opinion is generally too much to put on >> any stack.) > > Even in tests? You have a point, but why do things differently in tests for no reason? (So I don't leak memory, yeah, sure, but why not write the test in Rust then from the start? O:-P) >> But why would you want to put it on the stack anyway? It's not like it >> would make the patch any simpler. > > If you like the patch as is, go merge it as is :) W-well, I wasn't CC-d! I take this as a request to take this one patch from the series and merge it into my tree, so I will do that. O:-) Max signature.asc Description: OpenPGP digital signature
Re: [Qemu-block] [Qemu-devel] [PATCH 2/4] tests: fix bdrv-drain leak
Max Reitz writes: > On 2018-08-10 08:28, Markus Armbruster wrote: >> Marc-André Lureau writes: >> >>> Spotted by ASAN: >>> >>> = >>> ==5378==ERROR: LeakSanitizer: detected memory leaks >>> >>> Direct leak of 65536 byte(s) in 1 object(s) allocated from: >>> #0 0x7f788f83bc48 in malloc (/lib64/libasan.so.5+0xeec48) >>> #1 0x7f788c9923c5 in g_malloc (/lib64/libglib-2.0.so.0+0x523c5) >>> #2 0x5622a1fe37bc in coroutine_trampoline >>> /home/elmarco/src/qq/util/coroutine-ucontext.c:116 >>> #3 0x7f788a15d75f in __correctly_grouped_prefixwc >>> (/lib64/libc.so.6+0x4c75f) >>> >>> Signed-off-by: Marc-André Lureau >>> --- >>> tests/test-bdrv-drain.c | 1 + >>> 1 file changed, 1 insertion(+) >>> >>> diff --git a/tests/test-bdrv-drain.c b/tests/test-bdrv-drain.c >>> index 17bb8508ae..abc8bbe6f0 100644 >>> --- a/tests/test-bdrv-drain.c >>> +++ b/tests/test-bdrv-drain.c >>> @@ -948,6 +948,7 @@ static void coroutine_fn test_co_delete_by_drain(void >>> *opaque) >>> } >>> >>> dbdd->done = true; >>> +g_free(buffer); >>> } >>> >>> /** >> >> Screwed up in commit 4c8158e359d. >> >> Max, could the coroutine's stack accomodate the buffer? > > I don't know. How big is a coroutine's stack? The buffer size > apparently is 64k. (Which is my opinion is generally too much to put on > any stack.) Even in tests? > But why would you want to put it on the stack anyway? It's not like it > would make the patch any simpler. If you like the patch as is, go merge it as is :)
Re: [Qemu-block] [Qemu-devel] [PATCH 2/4] tests: fix bdrv-drain leak
On 2018-08-10 08:28, Markus Armbruster wrote: > Marc-André Lureau writes: > >> Spotted by ASAN: >> >> = >> ==5378==ERROR: LeakSanitizer: detected memory leaks >> >> Direct leak of 65536 byte(s) in 1 object(s) allocated from: >> #0 0x7f788f83bc48 in malloc (/lib64/libasan.so.5+0xeec48) >> #1 0x7f788c9923c5 in g_malloc (/lib64/libglib-2.0.so.0+0x523c5) >> #2 0x5622a1fe37bc in coroutine_trampoline >> /home/elmarco/src/qq/util/coroutine-ucontext.c:116 >> #3 0x7f788a15d75f in __correctly_grouped_prefixwc >> (/lib64/libc.so.6+0x4c75f) >> >> Signed-off-by: Marc-André Lureau >> --- >> tests/test-bdrv-drain.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/tests/test-bdrv-drain.c b/tests/test-bdrv-drain.c >> index 17bb8508ae..abc8bbe6f0 100644 >> --- a/tests/test-bdrv-drain.c >> +++ b/tests/test-bdrv-drain.c >> @@ -948,6 +948,7 @@ static void coroutine_fn test_co_delete_by_drain(void >> *opaque) >> } >> >> dbdd->done = true; >> +g_free(buffer); >> } >> >> /** > > Screwed up in commit 4c8158e359d. > > Max, could the coroutine's stack accomodate the buffer? I don't know. How big is a coroutine's stack? The buffer size apparently is 64k. (Which is my opinion is generally too much to put on any stack.) But why would you want to put it on the stack anyway? It's not like it would make the patch any simpler. Max signature.asc Description: OpenPGP digital signature
Re: [Qemu-block] [Qemu-devel] [PATCH 2/4] tests: fix bdrv-drain leak
Marc-André Lureau writes: > Spotted by ASAN: > > = > ==5378==ERROR: LeakSanitizer: detected memory leaks > > Direct leak of 65536 byte(s) in 1 object(s) allocated from: > #0 0x7f788f83bc48 in malloc (/lib64/libasan.so.5+0xeec48) > #1 0x7f788c9923c5 in g_malloc (/lib64/libglib-2.0.so.0+0x523c5) > #2 0x5622a1fe37bc in coroutine_trampoline > /home/elmarco/src/qq/util/coroutine-ucontext.c:116 > #3 0x7f788a15d75f in __correctly_grouped_prefixwc > (/lib64/libc.so.6+0x4c75f) > > Signed-off-by: Marc-André Lureau > --- > tests/test-bdrv-drain.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/tests/test-bdrv-drain.c b/tests/test-bdrv-drain.c > index 17bb8508ae..abc8bbe6f0 100644 > --- a/tests/test-bdrv-drain.c > +++ b/tests/test-bdrv-drain.c > @@ -948,6 +948,7 @@ static void coroutine_fn test_co_delete_by_drain(void > *opaque) > } > > dbdd->done = true; > +g_free(buffer); > } > > /** Screwed up in commit 4c8158e359d. Max, could the coroutine's stack accomodate the buffer?
