Re: [Qemu-devel] [PATCH 2/4] tests: fix bdrv-drain leak

[Max Reitz]

On 2018-08-13 08:15, Markus Armbruster wrote:
> Max Reitz <address@hidden> writes:
> 
>> On 2018-08-10 08:28, Markus Armbruster wrote:
>>> Marc-André Lureau <address@hidden> 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 <address@hidden>
>>>> ---
>>>>  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