Newsgroups: php.internals Path: news.php.net Xref: news.php.net php.internals:71069 Return-Path: Mailing-List: contact internals-help@lists.php.net; run by ezmlm Delivered-To: mailing list internals@lists.php.net Received: (qmail 6204 invoked from network); 11 Jan 2014 16:53:41 -0000 Received: from unknown (HELO lists.php.net) (127.0.0.1) by localhost with SMTP; 11 Jan 2014 16:53:41 -0000 Authentication-Results: pb1.pair.com smtp.mail=ab@php.net; spf=unknown; sender-id=unknown Authentication-Results: pb1.pair.com header.from=ab@php.net; 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:56674] helo=klapt.com) by pb1.pair.com (ecelerity 2.1.1.9-wez r(12769M)) with ESMTP id 36/D0-01384-31771D25 for ; Sat, 11 Jan 2014 11:53:40 -0500 Received: by klapt.com (Postfix, from userid 33) id 7EEE523D6080; Sat, 11 Jan 2014 17:53:36 +0100 (CET) Received: from 188.110.167.245 (SquirrelMail authenticated user anatol@belski.net) by webmail.klapt.com with HTTP; Sat, 11 Jan 2014 17:53:36 +0100 Message-ID: In-Reply-To: References: <10f19e40b03ee19837010a988eb05706.squirrel@webmail.klapt.com> <2e9df2840916d6e72255764a08086e57.squirrel@webmail.klapt.com> Date: Sat, 11 Jan 2014 17:53:36 +0100 To: "Jakub Zelenka" Cc: "Anatol Belski" , "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 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. > The another thing is that this checking needs to be done by other > libraries wrappers after getting parameters because the most libraries > using smaller types. That was exactly what I did when I worked on openssl > ext for 64bit and I will need to do it in my fann and crypto extension. > There are bunch > of other extensions that will need to do it too (for example imagick). The > difference with openssl changes is that the BC will need to be kept. I > am sure that if warnings were already in zend_parse_arg_impl, then the > work for supporting 64bit branch would much easier for me and other PECL > extension maintainers... > Yes, that's the case - the range checks have to be done locally according to what the wrapped library needs. If you haven't done them till now, you might have an issue on 64 bit linux anyway. Like say the lib uses int, but 64 bit long is passed. If that checks are already in place in your ext, then it's fine. The range checks you've done for openssl are the obligatory part, that's what I was doing as well for all the other extensions. Some are still pending as on the progress wiki page. And of course this is to mention in the porting guide. The libraries are different, for instance iconv uses size_t for string length, libxml uses int for string length. zpp cannot know, which library an extension uses. On the other hand - an extension without any library deps but with some math functions should be kept in mind, too. Regards anatol