Skip to content

fix incorrect usage of zend_declare_class_constant_ex#1868

Merged
sergeyklay merged 3 commits intozephir-lang:developmentfrom
dzuelke:fix-phalcon-14160
Jun 7, 2019
Merged

fix incorrect usage of zend_declare_class_constant_ex#1868
sergeyklay merged 3 commits intozephir-lang:developmentfrom
dzuelke:fix-phalcon-14160

Conversation

@dzuelke
Copy link
Copy Markdown
Contributor

@dzuelke dzuelke commented Jun 6, 2019

phalcon/cphalcon#14160
https://bugs.php.net/bug.php?id=78121

Hello!

This fixes phalcon/cphalcon#14160, author attribution is for @krakjoe in the commit.

I haven't touched the changelog yet as I'm not sure what the target version would be? Surely a bugfix release and not the next major version?

PR is editable.

@krakjoe
Copy link
Copy Markdown
Contributor

krakjoe commented Jun 6, 2019

The build will fail in 7.1, the string only needs to be interned from 7.2+, the API used doesn't exist in 7.1 ... sorry about that, I wasn't meaning for the patch to be used verbatim, just showing the way ;)

@krakjoe
Copy link
Copy Markdown
Contributor

krakjoe commented Jun 6, 2019

diff --git a/ext/kernel/main.c b/ext/kernel/main.c
index c940e0e26..c8d45ce0b 100644
--- a/ext/kernel/main.c
+++ b/ext/kernel/main.c
@@ -387,7 +387,24 @@ zend_class_entry* zephir_get_internal_ce(const char *class_name, unsigned int cl
 /* Declare constants */
 int zephir_declare_class_constant(zend_class_entry *ce, const char *name, size_t name_length, zval *value)
 {
-#if PHP_VERSION_ID >= 70100
+#if PHP_VERSION_ID >= 70200
+       int ret;
+       zend_string *key;
+
+       if (ce->type == ZEND_INTERNAL_CLASS) {
+               key = zend_string_init_interned(name, name_length, 1);
+       } else {
+               key = zend_string_init(name, name_length, 0);
+       }
+
+       zend_declare_class_constant_ex(ce, key, value, ZEND_ACC_PUBLIC, NULL);
+
+       if (ce->type != ZEND_INTERNAL_CLASS) {
+               zend_string_release(key);
+       }
+
+       return ret;
+#elif PHP_VERSION_ID >= 70100
        int ret;
 
        zend_string *key = zend_string_init(name, name_length, ce->type & ZEND_INTERNAL_CLASS);

Try that ... (this one can be used verbatim) ...

@krakjoe
Copy link
Copy Markdown
Contributor

krakjoe commented Jun 6, 2019

I'm confused, we were talking on a cphalcon issue, and now here ... the patch above was made against cphalcon, I can't figure out where to send a full pr, and that doesn't seem to apply here ...

@dzuelke
Copy link
Copy Markdown
Contributor Author

dzuelke commented Jun 6, 2019

Zephir is the "framework" with which Phalcon is written; the ext/kernel/main.c is vendored in from Zephir, and there the file in turn is generated from kernels/ZendEngine3/main.c.

@krakjoe
Copy link
Copy Markdown
Contributor

krakjoe commented Jun 6, 2019

Oh I see, I opened a pr on both anyway, wanted to see a green build ;)

So, I should close the cphalcon pr ?

@dzuelke
Copy link
Copy Markdown
Contributor Author

dzuelke commented Jun 6, 2019

There, @krakjoe, next attempt (diff looks a bit ugly on GitHub).

The important bit is to not edit ext/kernel/main.c directly it seems (I'm not a Phalcon user either, just trying to help fix the segfault a customer is having on Heroku).

@krakjoe
Copy link
Copy Markdown
Contributor

krakjoe commented Jun 6, 2019 via email

@Jurigag Jurigag requested review from dreamsxin and sergeyklay June 7, 2019 06:22
@sergeyklay sergeyklay merged commit d0ec39d into zephir-lang:development Jun 7, 2019
@sergeyklay
Copy link
Copy Markdown
Contributor

Thank you!

@dzuelke dzuelke mentioned this pull request Jun 7, 2019
sergeyklay added a commit that referenced this pull request Jun 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3.4.3 segfaults with PHP 7.3.6 (but not 7.3.5) and OPcache

3 participants