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

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. > > 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. > > 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. What do you think? Cheers Jakub