Use zend_string* for op_array->arg_info[].name and class_name

Hey:

Hi,

Please review the patch
https://gist.github.com/dstogov/47a39aff37f0a6441ea0

I am not sure, why can't we build a zend_string arg->name from char *
while doing register internal functions?

We can, but it'll take significant amount of additional memory and then we
will have to free it on shutdown.

Thanks. Dmitry.

then we can have the same zend_arg_info in runtime for both
internal/user functions?

thanks

Thanks. Dmitry.

--
Xinchen Hui
@Laruence
http://www.laruence.com/

Hi Nikita,

Thanks for review. I already thought about both approaches and failed as
well (the second also doesn't work with C++).
The proposed patch doesn't complicate engine a lot (may be only the
inheritance code), but I afraid about problems in some edge cases.

Thanks. Dmitry.

Hi,

Please review the patch
https://gist.github.com/dstogov/47a39aff37f0a6441ea0

Thanks. Dmitry.

Hi Dmitry, sorry for late reply.

The problem we're trying to solve here is lack of ability to create a
zend_string at compile time. I just tried two ideas how we might be able to
do that, to avoid introducing different arginfos for internal/userland
functions.

My first approach was to create a zend_string as a string literal using
(zend_string *) ("\1\0\0\0" "\6\1\0\0" "\0\0\0\0" "\0\0\0\0" str) (for
32bit LE) and then update the length in zend_register_functions. However
this didn't work because the string literal ends up in readonly
memprotected memory, so this causes a segfault.

My second approach was to use a C99 compound literal to create a temporary
zend_string-like structure with the correct length:

#define ZEND_STRING_CT(str)
(zend_string *) (struct {
uint32_t refcount; uint32_t type_info;
zend_ulong h; size_t len;
char val[sizeof(str)];
}[1]) {{ 1, IS_STRING | (IS_STR_PERSISTENT << 8), 0, sizeof(str)-1,
str }}

This seems to work fine (patch
https://github.com/nikic/php-src/commit/5d49321cd9728e0cc1c2939432e46159f9a78472).
However it requires C99, which we're currently not allowed to use.

Maybe someone has an idea how this can be done in C89?

Nikita

Hi Levi,

I understood, that you don't see a big problems with the patch. :)

According to your suggestions, It's possible to do it in this way, but it's
going to be slower, because it would require additional memory allocation,
but I agree that duplicating logic in the code isn't good.
It must be better to write something like the following (I didn't test it,
but it'll work):

I'll update the patch.

Thanks. Dmitry.

char *class_name;
size_t class_name_len;

if (fe->type == ZEND_INTERNAL_FUNCTION) {
class_name = ((zend_internal_arg_info*)fe_
arg_info)->class_name;
class_name_len = strlen(class_name);
} else {
class_name = fe_arg_info->class_name->val;
class_name_len = fe_arg_info->
class_name->len;
}

if (class_name_len == sizeof("parent")-1 &&
!zend_binary_strcasecmp(class_name, "parent", sizeof("parent")-1,
sizeof("parent")-1) &&
proto->common.scope) {
fe_class_name = zend_string_copy(proto->common.scope->name);
} else if (class_name_len == sizeof("self")-1 &&
!zend_binary_strcasecmp(class_name, "self", sizeof("self")-1,
sizeof("self")-1) &&
fe->common.scope) {
fe_class_name = zend_string_copy(fe->common.scope->name);
} else {
fe_class_name = class_name;
}

Please review the patch
https://gist.github.com/dstogov/47a39aff37f0a6441ea0

Instead of duplicating logic in two places I'd rather grab the
relevant data first, then do the logic once:

zend_string class_name;
if (fe->type == ZEND_INTERNAL_FUNCTION) {
class_name = zend_string_init(
((zend_internal_arg_info)fe_arg_info)->class_name,
strlen(((zend_internal_arg_info*)fe_arg_info)->class_name), 0
);
} else {
class_name = zend_string_dup(fe_arg_info->class_name);
}

if (!zend_string_equals_literal_ci(class_name, "parent") &&
proto->common.scope) {
fe_class_name = zend_string_copy(proto->common.scope->name);
} else if (!zend_string_equals_literal_ci(class_name, "self") &&
fe->common.scope) {
fe_class_name = zend_string_copy(fe->common.scope->name);
} else {
fe_class_name = class_name;
}
zend_string_release(class_name);