kjdev / php-ext-zstd

Zstd Extension for PHP
MIT License
201 stars 27 forks source link

Don't free output string if it must be returned #4

Closed sergey-dryabzhinsky closed 6 years ago

sergey-dryabzhinsky commented 6 years ago

It may cause SEGFAULT. Free output, rBuff, cBuff only if error happens or RETURN_FALSE.

sergey-dryabzhinsky commented 6 years ago

I'm sorry for disturbing but it not closed yet: In the end of functions you are using macros RETVAL_ which don't do return. There should be RETURN_ macros. https://github.com/kjdev/php-ext-zstd/blob/master/zstd.c#L118 https://github.com/kjdev/php-ext-zstd/blob/master/zstd.c#L160 https://github.com/kjdev/php-ext-zstd/blob/master/zstd.c#L221 https://github.com/kjdev/php-ext-zstd/blob/master/zstd.c#L285

Because of that following efree will do mess with data. ref: https://github.com/ryantenney/php7/blob/master/Zend/zend_API.h#L598

sergey-dryabzhinsky commented 6 years ago

Or you need to use duplicate flag always. Seems like these checks incorrect: https://github.com/kjdev/php-ext-zstd/blob/master/zstd.c#L117 That macro is present in all versions of PHP (4.4 - 7.1): https://github.com/kjdev/php-ext-zstd/blob/master/zstd.c#L120

sergey-dryabzhinsky commented 6 years ago

I'll try to make PR

sergey-dryabzhinsky commented 6 years ago

Seems all works fine. No SEGFAULTs now. Sorry for misleading comments.

ahmer-aq commented 6 years ago

@sergey-dryabzhinsky I need help, I was struggling from last 2 week just to understand how to generate .dll of php-ext-zstd on windows, if that possible you can provide the .dll so that I can start working on project. If it's possible can you guys write a small wiki guide on how to make php-ext-zstd up and running on windows, it would be a great help. here is my email ahmer.aq@gmail.com, if it's possible can you send me the .dll or any guidence I will be very thankfull to you.