Hi Jakub,
Hi Anatol,
Jakub
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:
- 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