Re: [RFC] 64 bit platform improvements for string length and integer

From: Date: Tue, 06 May 2014 09:13:33 +0000
Subject: Re: [RFC] 64 bit platform improvements for string length and integer
References: 1  Groups: php.internals 
Request: Send a blank email to internals+get-73958@lists.php.net to get a copy of this message
On Tue, 6 May 2014, Anatol Belski wrote:

> I would sincerely invite everyone to the resurrected 
> str_size_and_int64 RFC discussion.
> 
> I propose the discussion to last for one week as allowed by the voting 
> RFC because this topic has already been discussed to death previously 
> (any objections?). As no userland change is introduced, the discussion 
> would end and the voting would start on May 13th with 50%+1 votes 
> requirement.
> 
> https://wiki.php.net/rfc/size_t_and_int64_next

I see this still says "The usage of long datatype continues on 32 bit 
platforms." — do I understand correctly that on 32bit platforms, there 
won't still be a 64bit integer in PHP and that this "only" makes 64bit 
integers work on the Windows LLP64 model? Or are they still 32bit 
integers there?

Under "Accepting values with zend_parse_parameters()"

“l” 	“i” 	to accept integer argument, the internal var has to be declared as php_int_t
(inside PHP) or zend_int_t (inside Zend)
“L” 	“I” 	to accept integer argument with range check, the internal var has to be declared
as php_int_t (inside PHP) or zend_int_t (inside Zend)

Using l and I is going to be major confusing and incredibly hard to 
spot.

"'l', 'L', 's', 'p' parameter formats aren't
available anymore"

That means that every zpp call now needs to have an #ifdef around it to 
support post-this-patch and pre-this-patch PHP versions, as well as 
around their variable declarations. I'm afraid that will result in a 
huge mess, so we should look at this again.

Under "Example on accepting parameters with zpp"

You have:
	zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "iISP"

But why is both i and I allowed (or did I misread the Old/New before?) 
If so, then my comment on i, l, I is already valid :-)

Under "Example on printf specs usage"

You have:
	php_error_docref(NULL TSRMLS_CC, E_WARNING, "Value '" ZEND_INT_FMT "' is
out of range", i0);

Using that macro there feels really unnatural, and many C developers 
will not bother to figure out that they use that, of course ,because 
they are familiar with "%d" and "%l". 

And similarly under "Example on printf specs usage (no BC)" for:

	"Value '%pd' is out of range", i0

The new macros that you use in the example for "char *dup_substr(zval 
*s, zval *i)" also infer that all those accesses need to have a big 
#ifdef around stuff to support multiple versions. Again, making it a 
major pain for extension developers to support pre- and post-patch PHP 
versions.

I'm afraid with all those concerns, I will have to vote "no".

cheers,
Derick

-- 
http://derickrethans.nl | http://xdebug.org
Like Xdebug? Consider a donation: http://xdebug.org/donate.php
twitter: @derickr and @xdebug
Posted with an email client that doesn't mangle email: alpine


Thread (11 messages)

« previous php.internals (#73958) next »