search

bendi / node-mpg123n

mpg123 bindings that compile and run under MSVC++
4 stars 5 forks source link

Handle leak; memory leak #11

Open ntfshard opened 7 years ago

ntfshard commented 7 years ago

Hello

I guess in _master/src/nodempg123.cc in line: 51 your _mpg123handle *mh leaks; IMHO you should destroy it in dtor with _mpg123_delete(mpg123handle *);

BTW it looks like your _loopdata pointer from line: 60 doesn't have a corresponding delete IMHO dynamic allocation is not needed here =)

P.S. you can delete/free a nullptr -- it's ok (according to C++ standard): n3797:5.3.5_2 ... In the first alternative (delete object), the value of the operand of delete may be a null pointer value, a pointer to a non-array object created by a previous new-expression, or a pointer to a subobject...

So you can avoid using condition like in line 122

--M

bendi commented 7 years ago

Great thanks for pointing it out! Would you have time to prepare pull request?

ntfshard commented 7 years ago

Ok, I'll fix it; Unfortunately I'm not a nodejs user and it could be a little bit difficult to test it properly

bendi commented 7 years ago

Great!

Maybe you can use sample player in examples directory for testing?

ntfshard commented 7 years ago

This patch should fix memory leak. Unfortunately I found same issues in a bendi/node-mp3info project =\

Happy holidays