Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:71112
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: unknown (pb1.pair.com: domain php.net does not designate 85.214.73.107 as permitted sender)
Message-ID: <68ce1fbf9527dd9b40c0df9f28229bee.squirrel@webmail.klapt.com>
Date: Mon, 13 Jan 2014 21:58:16 +0100
To: "Jakub Zelenka" <bukka@php.net>
Cc: "Nikita Popov""" <nikita.ppv@gmail.com>,
 "PHP Developers Mailing List" <internals@lists.php.net>
Reply-To: "Anatol Belski" <ab@php.net>
User-Agent: SquirrelMail/1.5.2 [SVN]
MIME-Version: 1.0
Content-Type: text/plain;charset=UTF-8
Content-Transfer-Encoding: 8bit
Subject: Re: [PHP-DEV] [RFC] 64 bit platform improvements for string length
 and integer
From: ab@php.net ("Anatol Belski")

Hi Jakub,

On Mon, January 13, 2014 00:23, Jakub Zelenka wrote:
> On Sun, Jan 12, 2014 at 9:18 PM, Jakub Zelenka <bukka@php.net> wrote:
>
>
>> Hi Anatol,
>>
>>
>> Jakub
>> On Sat, Jan 11, 2014 at 7:34 PM, Anatol Belski <ab@php.net> wrote:
>>
>>
>>> If a library expects long, in the new code that's the issue on 32 bit
>>>  windows only. So yes, probably the way you describe is plausible,
>>> check PHP_WIN32 and PHP_API_VERSION. Honestly, right at the place
>>> where I sit, I can't remember any library working with long (well,
>>> timeval struct and so on, not really libs). There are int, size_t,
>>> int64_t, ... so while the case you describe is of course possible,
>>> it's rather an exception. Usually a simple runtime range check will be
>>> good enough, if needed at all.
>>>
>>>
>> Actually there is one big library where you can find it. It's OpenSSL
>> which is why I was asking about it.. :) There are quite a few places
>> where it's used. The main ones are following:
>>
>> - ASN.1 API functions (I plan to completely wrap it :) ) where it's
>> used for data length. - BIO_ctrl for length parameter (BIO_ctrl is a
>> definition result for many macros like BIO_set_mem_buf,
>> BIO_set_buffer_size, BIO_set_write_buf_size...)
>> - Big numbers - there is a situation a bit more complicated but unsigned
>>  long is used (there will be necessary some other checks anyway so it's
>> not an issue)
>>
>> I think that would be good to have a look to the other exts and double
>> check if used libs have long parameters for data lengths. If not, then I
>>  agree with you that there is no point to do casting to long with
>> warnings just because of OpenSSL.
>>
That openssl part looks like not implemented in PHP. Talking about the
core lib deps as well as a lot of libs for PECL libraries, that's just not
the case. The case with BIO_ctrl does therefore not justify such global
warnings.

>> However it would be good to define in compat header a macro for
>> checking that long != php_int_t . I know that it's currently only _WIN64
>> but if you implement enabling 64bit on 32bit platform (future scope in
>> the RFC :) ), then there will be an extra definition.
>>
>>
That compat.h header isn't thought to be swiss army knife, once copied
into some extensions source, it of course can and should be adapted to the
concrete circumstances.

> I was thinking a bit more what will be the consequences of asn1 and bio
> in my ext for "l" flag and it won't be such a big issue regarding to zpp.
> There are just few cases in asn1 (ASN1_INTEGER_set for example) and BIO
> stuff will probably require mainly string conversions which is a bit
> different issue...
>
> What's the actually plan for keeping BC for "s" and "p"? Is it gonna be
> zend_str_size from compat header and "s" and "p" aliases of "S" and "P"?
>
> Something like...
> ...
> char *s; zend_str_size len; if (zend_parse_parameters(ZEND_NUM_ARGS()
> TSRMLS_CC, "s", &s, &len) ==
> FAILURE) { return; }
> ...
>
>
> If so, I think that that would be a slightly bigger issue (size_t vs
> int)...
>
> There are only two solutions of zpp "s" that I think of:
> 1. check range (INT_MAX) and cast it to int (possibly throw warning if
> it's out of range) -> that could result too many warning but it's safe 2.
> PHP version variable zend_str_size (see above) -> the same as discussed
> for long but considerably more places where we will need to find out what
> the zend_str_size really is.
>
Jakub, please read the RFC, there's no zend_str_size anymore. It was used
only while porting a lot of stuff. Also, there are now the tools and the
doc I was talking about
http://git.php.net/?p=php-src.git;a=tree;f=compat;hb=refs/heads/str_size_and_int64
. Yet raw, will improve with usage as there will questions come.

Regards

Anatol