[PHP-DEV] Fwd: Bug #55214 [Com]: use of __CLASS__ within trait returns trait name not class name

2011-07-24 Thread Stefan Marr
Hi:

I would like to ask for a review of the solution for the following issue.
A patch is currently available at:
https://bugs.php.net/patch-display.php?bug=55214patch=__CLASS__-in-traits.002.patchrevision=1311532096

The problem is that __CLASS__ used in the body of a trait method does
not behave as expected based on the advertised metaphor that traits
are some kind of compiler-assisted copy'n'past.

This patch provides this semantic, thus, __CLASS__ will correspond to
the class name of the class where the trait is actually used.

However, my solution is very hackish: I changed the implementation of
how __CLASS__ is handled, and instead of setting the class name on the
zval, I set the zval to IS_NULL, and one of the unused data members as
a flag identifying this NULL value to be special.

This flag is later checked when the methods are actually copied into
the class, and the zval is replaced by the actual expected class name.
To be able to garbage collect the created zval later, it is included
in the list of literals of the method.

However, I do not have any clue on what kind of invariants are
associated to literals, nor what there actual semantics is supposed to
be.
This is mainly because of our implicit policy of keeping the engine
code free from any comments.

Thus, I would really like a comment on this patch, otherwise I will
commit it next week and also add the missing __TRAIT__ magic variable
to mirror the __CLASS__ behavior in a trait.

Thanks a lot
Stefan

PS: just noted there might be at least one more bug. note to self: add
a test for a traits that is used by a trait that is used by a class


 Original Message 
Edit report at https://bugs.php.net/bug.php?id=55214edit=1

 ID:                 55214
 Comment by:         g...@php.net
 Reported by:        chris dot rutledge at gmail dot com
 Summary:            use of __CLASS__ within trait returns trait name not
                    class name
 Status:             Assigned
 Type:               Bug
 Package:            Scripting Engine problem
 Operating System:   Ubuntu
 PHP Version:        5.4.0alpha1
 Assigned To:        gron
 Block user comment: N
 Private report:     N

 New Comment:

Ok, updated the patch and would like to ask for a review.
This is still hacky, but now I use the literals of a function to be
able to clean up the zval for the __CLASS__ name. Thus, the memory
leak should be fixed.

Think we will still need a __TRAIT__ to mirror __CLASS__ and to get
the trait name itself when that is required.

The test case is missing in the patch:

--TEST--
Bug #55214 (Use of __CLASS__ within trait returns trait name not class name)
--FILE--
?php

trait ATrait {
 public static function get_class_name() {
   return __CLASS__;
 }

 public function get_class_name_obj() {
   return __CLASS__;
 }
}


class SomeClass {
  use ATrait;
}

$r = SomeClass::get_class_name();
var_dump($r);

$o = new SomeClass();
$r = $o-get_class_name_obj();
var_dump($r);

?
--EXPECT--
string(9) SomeClass
string(9) SomeClass


Previous Comments:

[2011-07-24 18:28:16] g...@php.net

The following patch has been added/updated:

Patch Name: __CLASS__-in-traits.002.patch
Revision:   1311532096
URL:        
https://bugs.php.net/patch-display.php?bug=55214patch=__CLASS__-in-traits.002.patchrevision=1311532096


[2011-07-23 17:53:25] g...@php.net

I attached a patch of how a fix could be done.

I have to admit that it is hacky, but I think this is the expected
behavior with respect to the metaphor of compiler assisted
copy'n'past.

However, the patch is problematic, since it introduces a new memory leak.
And I do not have a good strategy yet to fix it.

Not sure how another patch could work, but the general idea is that
__CLASS__ is not actually defined inside a trait (BTW: we should add
__TRAIT__, too, yes).
Thus, it resolves to a IS_NULL value.
And as it happens to be, IS_NULL makes all is data members invalid,
and I use that to indicate that it isn't actually a NULL value but
that I want to fix it up with the class name when the method is
actually flattened/copied into the class.

The problem with the memory leak comes from the fact that copying the
method is not actually done deeply but shallow. And, I do not know how
to free only my fixed up names/ZVALs :-/.


[2011-07-23 17:45:28] g...@php.net

The following patch has been added/updated:

Patch Name: __CLASS__-in-traits.patch
Revision:   1311443128
URL:        
https://bugs.php.net/patch-display.php?bug=55214patch=__CLASS__-in-traits.patchrevision=1311443128


[2011-07-23 14:17:24] fel...@php.net

It's simple to add the __TRAIT__ one, just like __CLASS__ does.

But making a more magic __CLASS__ to reflect the class that called 

[PHP-DEV] Consider the use of rawurldecode() on $_GET and $_REQUEST instead of urldecode()

2011-07-24 Thread Tig
Current the following string (inside the quotes) give me a + plz
should be encoded correctly to give%20me%20a%20+%20plz when passed
as a query string.

Now when the query string is presented with $_GET['x'], the result is
give me a   plz. Clearly wrong.

This leads to people (I'm sure I'm not the only one) using crazy hacks
to get something as simple as a query string var.

rawurlencode('give me a + plz') will produce give%20me%20a%20%2B%20plz
which is not give%20me%20a%20+%20plz, but does not 'drop' the + when
someone types in the query string.

The following code can be used to show the error (lines may wrap):

?php

$str = 'give me a + plz';

echo '
a href='.$_SERVER['SCRIPT_NAME'].'?qs='.$str.'As typed by someone/abr /
a href='.$_SERVER['SCRIPT_NAME'].'?qs='.urlencode($str).'With
urlencode()/abr /
a href='.$_SERVER['SCRIPT_NAME'].'?qs='.rawurlencode($str).'With
rawurlencode()/abr /
';

if ($_GET['qs']) {
echo 'pResults:br /
'.$_SERVER['QUERY_STRING'].' #60; $_SERVER[QUERY_STRING]br /
'.$_GET['qs'].' #60; $_GET[qs]br /
'.urldecode($_SERVER['QUERY_STRING']).' #60;
urldecode($_SERVER[QUERY_STRING])br /
'.rawurldecode($_SERVER['QUERY_STRING']).' #60;
rawurldecode($_SERVER[QUERY_STRING])
/p';
// code to break up the query string because _GET[] is not
// correct - NOT 100% reliable as well!!
$tmp = explode('',$_SERVER['QUERY_STRING']);
foreach($tmp as $q) {
$qs = explode('=',$q);
for($i = 0; $i  count($qs) ; $i++) {
$real_GET[$qs[$i]] = rawurldecode($qs[++$i]);
}
} // foreach

echo 'p
What was really passed?? Impossible to tell for sure but less likely to
be what _GET has:br /'.$real_GET['qs'].'
/p';

} // _GET[qs]
?

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php



Re: [PHP-DEV] new gcov.php.net machine is up

2011-07-24 Thread Gwynne Raskind
On this subject, I've been looking into what produces the largest
warnings spam with a decent set of warnings turned on, and I'd like to
recommend this patch. I can't commit it myself (I don't have Zend
karma), nor would I care to without getting some opinion on it. The
patch is against 5.4, but should apply equally to trunk. No API is
changed, and no BC issues are created; it only adds a
forward-compatible optional declarator for the end of a
zend_function_entry struct. Obviously, the spammed warning in question
is missing initializer, with respect to every function entry struct
in every extension, all of which simply use the {NULL, NULL, NULL}
from ext_skel. The intention is to provide this constant to protect
against future extensions of the structure. There are some other
structures which could also benefit from such an initializer
(smart_str and zend_fcall_info[_cache] come to mind).

Index: Zend/zend_API.h
===
--- Zend/zend_API.h (revision 313656)
+++ Zend/zend_API.h (working copy)
@@ -96,6 +96,8 @@
 #define ZEND_NS_FALIAS(ns, name, alias, arg_info)  
ZEND_NS_FENTRY(ns,
name, ZEND_FN(alias), arg_info, 0)
 #define ZEND_NS_DEP_FALIAS(ns, name, alias,
arg_info)   ZEND_NS_FENTRY(ns, name, ZEND_FN(alias), arg_info,
ZEND_ACC_DEPRECATED)

+#define ZEND_FE_END{ NULL, NULL, NULL, 0, 0 }
+
 #define ZEND_ARG_INFO(pass_by_ref, name)   
{ #name,
sizeof(#name)-1, NULL, 0, 0, 0, pass_by_ref},
 #define ZEND_ARG_PASS_INFO(pass_by_ref)
{ NULL, 0, NULL, 0, 0,
0, pass_by_ref},
 #define ZEND_ARG_OBJ_INFO(pass_by_ref, name, classname, allow_null) {
#name, sizeof(#name)-1, #classname, sizeof(#classname)-1, IS_OBJECT,
allow_null, pass_by_ref},
Index: main/php.h
===
--- main/php.h  (revision 313656)
+++ main/php.h  (working copy)
@@ -359,6 +359,7 @@
 #define PHP_MALIAS  ZEND_MALIAS
 #define PHP_ABSTRACT_ME ZEND_ABSTRACT_ME
 #define PHP_ME_MAPPING  ZEND_ME_MAPPING
+#define PHP_FE_END ZEND_FE_END

 #define PHP_MODULE_STARTUP_N   ZEND_MODULE_STARTUP_N
 #define PHP_MODULE_SHUTDOWN_N  ZEND_MODULE_SHUTDOWN_N

-- Gwynne



On Sat, Jul 23, 2011 at 19:23, Rasmus Lerdorf syst...@php.net wrote:
 On 07/23/2011 04:07 PM, Gwynne Raskind wrote:
 Here's my question - if I made some smaller commits here and there to
 fix warnings in core, would that be accepted? I don't have time to do
 sweeping changes, but fixing one file today, a couple the next day,
 etc., is within my abilities (including making sure no regressions are
 introduced, of course).

 That's fine if it is done carefully. Note that the code needs to compile
 on many different platforms, on many different compilers and versions of
 compilers.

 -Rasmus



-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php