Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

More work on pecl-memcache #76

Merged
merged 15 commits into from Oct 16, 2020
Merged

More work on pecl-memcache #76

merged 15 commits into from Oct 16, 2020

Conversation

TysonAndre and others added 13 commits August 12, 2020 14:59
I think that these might have uninitialized bytes
when a data type larger than zend_bool is used for the ini setting,
if the module globals are set from malloc?
(Not 100% sure if the entire structure isn't set to 0 before being used)

Related to #56
Use zend_bool for ini bool settings
This will allow us to have one unified branch that will compile
under different versions of php.
- added / updated arginfos
- fix functions returning NULL instead of advertised return type,
  For example, when memcached returned error on "delete", function
  returned null instead of false
- added tests for both php8 and 7
@Zaffy Zaffy mentioned this pull request Oct 8, 2020
config.w32 Outdated Show resolved Hide resolved
Co-authored-by: Tyson Andre <tyson.andre@uwaterloo.ca>
@aidvu
Copy link
Contributor

aidvu commented Oct 30, 2020

Would be nice to have a test case covering the $flags parameter. I haven't tested on the PHP8 branch yet, but on the PHP7 branch when running PHP8 the $flags parameter isn't converted to an int on cache hits. It always returns false, making it impossible to distinguish between cache hit/miss.

Trying to get this ready for PHP8: https://github.com/Automattic/wp-memcached/blob/master/object-cache.php#L327

If I can help somehow, do tell.

@Zaffy
Copy link
Contributor Author

Zaffy commented Oct 30, 2020

You should be able to target both php8 and php7 with the php8 branch.
I just tried your little test on php8 branch and flags were intact on cache miss and set on cache hit.
Could you please try it on that branch? If the issue persist, please do consider making a reliable test case so it may be fixed.

@aidvu
Copy link
Contributor

aidvu commented Nov 5, 2020

Thanks @Zaffy. Tested with the php8 branch, working as expected. $flags are correctly set on cache-hit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment