Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:71945
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 209.85.217.171 as permitted sender)
MIME-Version: 1.0
In-Reply-To: <52ED5FDD.8030101@sugarcrm.com>
References: <e0681b3331094323d383a048471790f8.squirrel@webmail.klapt.com>
	<alpine.DEB.2.10.1401302346240.6067@whisky.home.derickrethans.nl>
	<52EAF181.3000907@sugarcrm.com>
	<CAEZPtU4XgaYk4N=vBOYBOQD65JiU1RRtxViWmn06SRb90-b8iA@mail.gmail.com>
	<52ED5FDD.8030101@sugarcrm.com>
Date: Sat, 1 Feb 2014 22:16:27 +0100
Message-ID: <CAEZPtU7X9=RajfUN+gxF7mE5N9GrWnLJUD_94dvXxRSvev_ixA@mail.gmail.com>
To: Stas Malyshev <smalyshev@sugarcrm.com>
Cc: Derick Rethans <derick@php.net>, Anatol Belski <ab@php.net>, 
	PHP Developers Mailing List <internals@lists.php.net>, Matt Ficken <mattficken@php.net>, 
	"Stephen A. Zarkos" <szarkos@php.net>
Content-Type: text/plain; charset=UTF-8
Subject: Re: [PHP-DEV] [VOTE] 64 bit platform improvements for string length
 and integer
From: pierre.php@gmail.com (Pierre Joye)

On Sat, Feb 1, 2014 at 9:58 PM, Stas Malyshev <smalyshev@sugarcrm.com> wrote:
> Hi!
>
>> There is a massive misunderstanding about the options provided in the votes.
>>
>> If the option #2 and #3 are accepted, only, I repeat, only the
>> declarations of the variables used by zpp have to be changed, nothing
>> else, no #ifdef, etc.
>
> I understand this. What is worrying me is that if you fail to do that
> for some function, you end up using wrong variable type, and it would
> lead to bugs that, from experience, are *very* hard to catch. If you
> break the code explicitly, it plants a huge red flag saying "Here! You
> need to fix this!". But if you just silently accept wrong types (and zpp
> has no way of doing typechecks) then your best red flag would come in
> the form of a user complaining "my PHP server crashes under the high
> load with PHP 5.6".
> I.e. if you do zpp("s", &char_ptr, &len) and zpp now expects len to be
> size_t but you keep it declared as int, no tool will alert you and
> you're putting 64-bit value into 32-bit stack slot.
>
>> It can be automated for zpp, it is almost the case already and Anatol
>> is working on this part to fully automate zpp changes (with option #2
>> and #3).
>
> The bad part is not zpp. zpp is the easy part.

Very easy even, the replace.php will take care of this part.

> The bad part is all vars
> outside zpp and keeping them all in sync.
>
>> I will prepare a sample ext compiling using 5.3, 5.4, 5.5 and with the
>> int64 branch, it will be a better example than what we can see in
>> mongodb, which does not use option #2 and #3. It is basically bad as
>> many of us only focus on this constellation and totally ignore these
>> options which were added as a possible very good compromise.
>
> I'm not ignoring that option. On the contrary, it is exactly that option
> that you describe that I am worried about. And I am worried about it
> exactly because since you don't have (meaning, forced by the compiler or
> by zpp failure) to make the changes in all functions, it's all too easy
> to miss one, and missing one is all to hard to figure out.


That's where the compiler warnings/errors come to the game. Except for
the situations where one does explicit bad casting, which should be
avoided in the 1st place, if possible.

That being said, I have been advocated warning free php-src for years
now. I think if we ever start php6 then we should really change our
policy and get clean codes. Any warning can produce undefined
behaviors or changing behaviors. Using gcc and VC as base should put
us on the safe side. Both gcc and vc latest versions have new options
to actually get rid of many of the issues you are describing easily.

Cheers,
-- 
Pierre

@pierrejoye | http://www.libgd.org