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;
}


On Fri, Nov 28, 2014 at 11:19 PM, Levi Morrison <le...@php.net> wrote:

> > 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);
>

Reply via email to