Newsgroups: php.internals Path: news.php.net Xref: news.php.net php.internals:71072 Return-Path: Mailing-List: contact internals-help@lists.php.net; run by ezmlm Delivered-To: mailing list internals@lists.php.net Received: (qmail 24287 invoked from network); 11 Jan 2014 19:35:06 -0000 Received: from unknown (HELO lists.php.net) (127.0.0.1) by localhost with SMTP; 11 Jan 2014 19:35:06 -0000 Authentication-Results: pb1.pair.com header.from=ab@php.net; sender-id=unknown Authentication-Results: pb1.pair.com smtp.mail=ab@php.net; spf=unknown; sender-id=unknown Received-SPF: unknown (pb1.pair.com: domain php.net does not designate 85.214.73.107 as permitted sender) X-PHP-List-Original-Sender: ab@php.net X-Host-Fingerprint: 85.214.73.107 klapt.com Received: from [85.214.73.107] ([85.214.73.107:58945] helo=klapt.com) by pb1.pair.com (ecelerity 2.1.1.9-wez r(12769M)) with ESMTP id 22/03-07897-6EC91D25 for ; Sat, 11 Jan 2014 14:35:05 -0500 Received: by klapt.com (Postfix, from userid 33) id D481F23D6080; Sat, 11 Jan 2014 20:34:59 +0100 (CET) Received: from 88.67.237.212 (SquirrelMail authenticated user anatol@belski.net) by webmail.klapt.com with HTTP; Sat, 11 Jan 2014 20:34:59 +0100 Message-ID: <09595ef77b25972feab8e047bebdd37c.squirrel@webmail.klapt.com> In-Reply-To: References: <10f19e40b03ee19837010a988eb05706.squirrel@webmail.klapt.com> <2e9df2840916d6e72255764a08086e57.squirrel@webmail.klapt.com> Date: Sat, 11 Jan 2014 20:34:59 +0100 To: "Jakub Zelenka" Cc: "Hannes Magnusson" , "Nikita Popov" , "PHP Developers Mailing List" Reply-To: "Anatol Belski" 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 Sat, January 11, 2014 19:31, Jakub Zelenka wrote: > On Sat, Jan 11, 2014 at 4:53 PM, Anatol Belski wrote: > > >> Hi Jakub, >> >> >> On Sat, January 11, 2014 17:08, Jakub Zelenka wrote: >> >>> On Sat, Jan 11, 2014 at 3:29 PM, Anatol Belski wrote: >>> >>> >>> >>>> >>>> yep, that's exactly how it performs till now in the mainstream, >>>> just replace ZEND_INT_MAX with LONG_MAX. Adding a warning to every >>>> such case would cause a warning flood in many PHP apps, guaranteed >>>> :) Whereby it >>>> might be ok for debug mode maybe, I wouldn't do it as I can't >>>> remember any WTFs about the behavior. >>>> >>>> >>>> >>> I meant adding warning only for overflow cases. Values in existing >>> apps can't be bigger than LONG_MAX so I don't see how it could cause >>> warning flood in the current PHP apps. I don't even think that users >>> will be often using such a big values after the 64bit changes. >>> >>> The thing is that if I use value bigger than LONG_MAX after 64bit >>> changes and I pass it to the function defined in extension that does >>> not support it (use "l" in zpp), then I rather see warning than >>> unexpected rounding >> to >>> LONG_MAX... There is no way how to find out (except looking to the >>> ext sources) that the big values are not supported. If I get warning, >>> I can >>> fix it in the code straight away... >>> >> But isn't it the exact situation now on ILP32 vs LP64 systems? Like 32 >> vs 64 bit Linux? I haven't suggested 'l' to handle the param the old >> way, just it to be an alias for 'i'. In the new code that would be as >> dynamic as php_int_t is, however 'l' as alias would retain zpp >> compatibility to the older PHP. >> >> For instance how much sense had such a centralized warding for the >> snippet is_int(PHP_INT_MAX+1) ? There is a lot of functions accepting an >> arbitrary numeric value and not depending on any library, why they >> should throw a warning by default? Why should we throw a warning about >> 32 bit in the 64 >> bit build? Furtehrmore, LONG_MAX is 64 bit on LP64 systems, that would >> be the same as PHP_INT_MAX. >> >> > This is of course just about LLP64 when the situation has changed. I > think that we are talking about slightly different things. Sorry my first > example was a bit incorrect (ZEND_INT_MAX = _I64_MAX which is bigger than > LONG_MAX > on LLP64) well, there's no another chance to understand each other than using some verbal devices :) > What I was suggesting is keeping BC for code like this: > ... > long n; if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "l", &n) == > FAILURE) { > return; } ... > The warning would be thrown if it's bigger than LONG_MAX or smaller than > LONG_MIN. > > If I understood correctly you are suggesting using php_int_t for n > (defined > in compat header as long if not defined in php.h)? The problem is that the > type would be dependent on PHP versions and when you need to be sure > that it's a long (which might be the case when you pass it to the another > lib function), then you need to check PHP_API_VERSION and in case it's not > long, you have to do range check. 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. > > In general, I think that would be better to keep BC for "l" and use it > just for long values (the "l" actually stands for long... :) ). That could > be achieved adding range checks with warning if the value is bigger than > LONG_MAX or smaller than LONG_MIN in zend_parse_arg_impl (it would > probably require small changes in zend_parse_arg [expected_value...]). > Just to remind, your very first reply was to the mail about the zpp compatibility, namely so then one wouldn't have to touch zpp calls for migration, only the variables. That's why i've suggested to alias the new formats with old. If we let 'l' to do the old thing, then zpp will have to be touched at many places to be replaced with 'i' with all the corresponding consequences. So retaining the functional compatibliity with 'l', an easier migration way is not possible. Also I'm asking myself, how essential it really is, in the light of how many libs will profit from one or another way. Regards anatol