Re: [PHP-DEV] [PATCH] - fix for 64 bit issues with OnUpdateInt
A single API is probably sufficient but I can understand that extension writers might want to use ints. The INI flag might have nothing to do with an internal zval and an int might be more than enough. I don't really mind but I wouldn't introduce OnUpdateInteger. I guess we should either move everything to OnUpdateLong() and nuke OnUpdateInt() in ZE2 or we change OnUpdateInt to work with ints and fix the whole code. It might be less confusing to just have one. I agree about the confusing part. My suggestion is: * we leave OnUpdateInt as is currently for BC, deprecating it now, removing it in (? php 5.x == ZE2 ?) * convert everything in the code base to OnUpdateLong, which means I change the arguments passed into the call where needed (yipee I get to do another code sweep :-) This fixes the symantic problem and sets the stage for how to proceed in the future. * update the documentation with a note reminding people that int and long are not the same thing, both for OnUpdateInt and for zend_parse_parameters (I found zend_parse_parameters in the docs but not OnUpdateInt, any hints ?) I *think* this would work with all of the comments I have seen on this issue. I will start coding today, and if people are ok with it I will commit on Friday (to allow time for comment) so that Jani can move on 4.3.2. dave
Re: [PHP-DEV] [PATCH] - fix for 64 bit issues with OnUpdateInt
On Fri, 28 Feb 2003, Andi Gutmans wrote: At 04:28 PM 2/28/2003 +0100, Sascha Schumann wrote: So I think the fix of adding OnUpdateLong() is the correct fix. I was under the impression that OnUpdateInt was actually expecting a long. I remember changing some int's to long's to address 64 bit issues. Do I remember this incorrectly? No, you are correct but misunderstood me. We should introduce OnUpdateLong() for ppl using longs and use OnUpdateInt() for ppl who want to use ints. So for 4.3.2, we add the OnUpdateLong() and replace all the calls to OnUpdateInt() to use that instead and we leave the OnUpdateInt() behaviour same as it was. This shouldn't cause BC problems then..? And in 5.0.0 we change OnUpdateInt() to use ints. --Jani -- PHP Development Mailing List http://www.php.net/ To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [PATCH] - fix for 64 bit issues with OnUpdateInt
So for 4.3.2, we add the OnUpdateLong() and replace all the calls to OnUpdateInt() to use that instead and we leave the OnUpdateInt() behaviour same as it was. This shouldn't cause BC problems then..? Yes. And in 5.0.0 we change OnUpdateInt() to use ints. No, unless you are into procuring stealth errors which go undetected for months. It would be better drop that interface completely. Although then extension maintainers whose code needs to work with several PHP releases will have to resort to preprocessor magic once again. - Sascha -- PHP Development Mailing List http://www.php.net/ To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [PATCH] - fix for 64 bit issues with OnUpdateInt
On Tue, 4 Mar 2003, Sascha Schumann wrote: So for 4.3.2, we add the OnUpdateLong() and replace all the calls to OnUpdateInt() to use that instead and we leave the OnUpdateInt() behaviour same as it was. This shouldn't cause BC problems then..? Yes. Ok, I'll prepare a patch for that. And in 5.0.0 we change OnUpdateInt() to use ints. No, unless you are into procuring stealth errors which go undetected for months. It would be better drop that interface completely. Although then extension maintainers whose code needs to work with several PHP releases will have to resort to preprocessor magic once again. Don't they have to do that anyway..? :) --Jani -- PHP Development Mailing List http://www.php.net/ To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [PATCH] - fix for 64 bit issues with OnUpdateInt
Don't they have to do that anyway..? :) No, why? For example, the session extension will be largely unchanged. The same code works in PHP 4 and 5. - Sascha -- PHP Development Mailing List http://www.php.net/ To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [PATCH] - fix for 64 bit issues with OnUpdateInt
So for 4.3.2, we add the OnUpdateLong() and replace all the calls to OnUpdateInt() to use that instead and we leave the OnUpdateInt() behaviour same as it was. This shouldn't cause BC problems then..? If you want to leave the current OnUpdateInt behavior (uses long * internally) ... then that will require some additional changes, as there are a dozen or so places where an int is currently passed to OnUpdateInt, which I left as is because I changed to using OnUpdateLong where a long was used. Dave -- PHP Development Mailing List http://www.php.net/ To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [PATCH] - fix for 64 bit issues with OnUpdateInt
Yes, I know, but we need to remain backwards compatible so I'm adding OnUpdateInteger() and OnUpdateLong(). This leaves OnUpdateInt() as it is now. Just need to go through the extensions and change the necessary OnUpdateInt()'s to OnUpdateLong(). --Jani On Tue, 4 Mar 2003, David Hill wrote: So for 4.3.2, we add the OnUpdateLong() and replace all the calls to OnUpdateInt() to use that instead and we leave the OnUpdateInt() behaviour same as it was. This shouldn't cause BC problems then..? If you want to leave the current OnUpdateInt behavior (uses long * internally) ... then that will require some additional changes, as there are a dozen or so places where an int is currently passed to OnUpdateInt, which I left as is because I changed to using OnUpdateLong where a long was used. Dave -- - For Sale! - -- PHP Development Mailing List http://www.php.net/ To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [PATCH] - fix for 64 bit issues with OnUpdateInt
And of course the rest to use OnUpdateInteger().. --Jani On Tue, 4 Mar 2003, Jani Taskinen wrote: Yes, I know, but we need to remain backwards compatible so I'm adding OnUpdateInteger() and OnUpdateLong(). This leaves OnUpdateInt() as it is now. Just need to go through the extensions and change the necessary OnUpdateInt()'s to OnUpdateLong(). --Jani On Tue, 4 Mar 2003, David Hill wrote: So for 4.3.2, we add the OnUpdateLong() and replace all the calls to OnUpdateInt() to use that instead and we leave the OnUpdateInt() behaviour same as it was. This shouldn't cause BC problems then..? If you want to leave the current OnUpdateInt behavior (uses long * internally) ... then that will require some additional changes, as there are a dozen or so places where an int is currently passed to OnUpdateInt, which I left as is because I changed to using OnUpdateLong where a long was used. Dave -- - For Sale! - -- PHP Development Mailing List http://www.php.net/ To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [PATCH] - fix for 64 bit issues with OnUpdateInt
The patch I sent in should have all of the OnUpdateLong changes in them, and the remaining OnUpdateInt should probably be OnUpDateInteger. The painful part was actually looking at what was passed to match up the calls. That might save you some time. dave hill Yes, I know, but we need to remain backwards compatible so I'm adding OnUpdateInteger() and OnUpdateLong(). This leaves OnUpdateInt() as it is now. Just need to go through the extensions and change the necessary OnUpdateInt()'s to OnUpdateLong(). -- PHP Development Mailing List http://www.php.net/ To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [PATCH] - fix for 64 bit issues with OnUpdateInt
Yup, that was the idea. I'll first change them all to OnUpdateInteger, and then use your patch to change the ones that need to be long to use OnUpdateLong. --Jani On Tue, 4 Mar 2003, David Hill wrote: The patch I sent in should have all of the OnUpdateLong changes in them, and the remaining OnUpdateInt should probably be OnUpDateInteger. The painful part was actually looking at what was passed to match up the calls. That might save you some time. dave hill Yes, I know, but we need to remain backwards compatible so I'm adding OnUpdateInteger() and OnUpdateLong(). This leaves OnUpdateInt() as it is now. Just need to go through the extensions and change the necessary OnUpdateInt()'s to OnUpdateLong(). -- - For Sale! - -- PHP Development Mailing List http://www.php.net/ To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [PATCH] - fix for 64 bit issues with OnUpdateInt
On Tue, 4 Mar 2003, Jani Taskinen wrote: Yup, that was the idea. I'll first change them all to OnUpdateInteger, and then use your patch to change the ones that need to be long to use OnUpdateLong. Is there any specific reason why a single API (OnUpdateLong) is not sufficient? Is not it a safe assumption that those modules which still use 'int's are simply the result of a mistake on the developer's side? - Sascha -- PHP Development Mailing List http://www.php.net/ To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [PATCH] - fix for 64 bit issues with OnUpdateInt
Is there any specific reason why a single API (OnUpdateLong) is not sufficient? Is not it a safe assumption that those modules which still use 'int's are simply the result of a mistake on the developer's side? This is a reasonable assumption actually I can't think of a case where a long would not work in place of an int. And given that you are talking of only a handfull that would be changed, size would not be an issue. Dave -- PHP Development Mailing List http://www.php.net/ To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [PATCH] - fix for 64 bit issues with OnUpdateInt
On Tue, 4 Mar 2003, David Hill wrote: Is there any specific reason why a single API (OnUpdateLong) is not sufficient? Is not it a safe assumption that those modules which still use 'int's are simply the result of a mistake on the developer's side? This is a reasonable assumption actually I can't think of a case where a long would not work in place of an int. And given that you are talking of only a handfull that would be changed, size would not be an issue. So just 'rename' the OnUpdateInt to OnUpdateLong and replace all OnUpdateInt with OnUpdateLong? But that doesn't fix anything? Blah..I'll leave this to David. :) --Jani -- PHP Development Mailing List http://www.php.net/ To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [PATCH] - fix for 64 bit issues with OnUpdateInt
At 11:03 PM 3/4/2003 +0100, Sascha Schumann wrote: On Tue, 4 Mar 2003, Jani Taskinen wrote: Yup, that was the idea. I'll first change them all to OnUpdateInteger, and then use your patch to change the ones that need to be long to use OnUpdateLong. Is there any specific reason why a single API (OnUpdateLong) is not sufficient? Is not it a safe assumption that those modules which still use 'int's are simply the result of a mistake on the developer's side? A single API is probably sufficient but I can understand that extension writers might want to use ints. The INI flag might have nothing to do with an internal zval and an int might be more than enough. I don't really mind but I wouldn't introduce OnUpdateInteger. I guess we should either move everything to OnUpdateLong() and nuke OnUpdateInt() in ZE2 or we change OnUpdateInt to work with ints and fix the whole code. It might be less confusing to just have one. Andi -- PHP Development Mailing List http://www.php.net/ To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [PATCH] - fix for 64 bit issues with OnUpdateInt
In message [EMAIL PROTECTED] on Wed, Mar 05, 2003 at 07:24:20AM +0200, Andi Gutmans wrote: It might be less confusing to just have one. Hasn't worked so far ;) -- PHP Development Mailing List http://www.php.net/ To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [PATCH] - fix for 64 bit issues with OnUpdateInt
At 01:29 PM 3/5/2003 +0800, James Devenish wrote: In message [EMAIL PROTECTED] on Wed, Mar 05, 2003 at 07:24:20AM +0200, Andi Gutmans wrote: It might be less confusing to just have one. Hasn't worked so far ;) Well we're talking about changing the name from OnUpdateInt to OnUpdateLong. It should reduce confusion. -- PHP Development Mailing List http://www.php.net/ To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [PATCH] - fix for 64 bit issues with OnUpdateInt
In message [EMAIL PROTECTED] on Wed, Mar 05, 2003 at 07:35:49AM +0200, Andi Gutmans wrote: At 01:29 PM 3/5/2003 +0800, James Devenish wrote: In message [EMAIL PROTECTED] on Wed, Mar 05, 2003 at 07:24:20AM +0200, Andi Gutmans wrote: Hasn't worked so far ;) Well we're talking about changing the name from OnUpdateInt to OnUpdateLong. It should reduce confusion. Hence the 'winking smiley'. But on the other hand, using 'long' in the zend_parse_parameters interface didn't prevent confusion. -- PHP Development Mailing List http://www.php.net/ To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [PATCH] - fix for 64 bit issues with OnUpdateInt
Hi, Preface: This e-mail uses the distribution list (To and CC addresses) that the original poster used. I have moved everyone other than php-dev to Bcc so that this doesn't get perpetuated (I have no idea of the significance of the other addresses that were e-mailed, nor do I know Dave Hill's standing in the PHP community). In message [EMAIL PROTECTED] on Thu, Feb 27, 2003 at 02:52:28PM -0500, Dave Hill wrote: Proposed Patch to address 64 bit issues in PHP v4.3.1 Diff -u against php4-STABLE-200302241430 Also needed in 4.5.x and 5.x Addresses bugs 20994, 21002, 21822, 20268 There was some discussion on php-dev in November 2002 after I posted patches for bug #20268 (the earliest-numbered one in Dave's list). I chose to address this problem by creating OnUpdateLong (in addition to to existing OnUpdateInt) and sweep though the code changing the call where needed to match the stucture item. An equally valid fix would be to change all of those longs to ints. (a) I also chose to create OnUpdateLong (and this has been working well for me since 4.3.0pre2 using the patches I posted). Since I posted those patches, PHP has progressed and more of these problems have been introduced (bad) but at the same time some of the test cases have been revised to be more portable (good). (b) I found that sometimes ints needed to stay as ints and longs needed to stay as longs, which is why I chose to separate OnUpdateLong from OnUpdateInt. (c) In discussions on php-dev in November, the subject of the thread was 64-bit PHP 4.3 (extensive long vs int problems) http://marc.theaimsgroup.com/?t=10369029101r=1w=2 (d) Jason Greene responded as a person who was working on this problem. Disfavour of the OnUpdateLong solution was expressed (on- and maybe off-list, too). After performing that sweep, I found there was a small number of other errors of the int/long type that my compiler found that I corrected and are included here. There might be more mix and match problems, I only addressed the ones in the modules I have enabled. Deja vu! Now the reason I am responding and the reason I have BCCed the people in Dave's list was that I wanted to mention something about Zend. Really, this bug has its roots in Zend though it is manifested in PHP modules. Firstly, OnUpdateInt assigns to long storage but uses zend_atoi -- clearly there is confusion already. But the more disturbing thing that I found in November was the Zend documentation for zend_parse_parameters (a site of extensive long/int problems). That is why I have written this e-mail. I know nothing about the Zend engine but I will refer to this document: http://www.zend.com/apidoc/zend.arguments.retrieval.php It is entitled Accepting Arguments / Retrieving Arguments and describes the zend_parse_parameters function. It lists a type specifier: l - long But it does not list an int type. In the actual code, it is entirely long-based, not int-based, so there is clearly a mismatch. But moreover, there is no acknowledgement of the significance of int versus long. So it is not surprising that so many developers have made mistakes arising from this. Therefore, I thought that a proper solution to this problem would have to involve some consideration of the Zend API (or at least copious acknowledgments and warnings in its documentation). There is potentially a huge developer education problem, too. That is why I chose to add OnUpdateLong. And given the choice, I would have 'i - int' added to the type specifiers of zend_parse_parameters. Firstly, having the OnUpdateLong function allows quick rectification of these problems without having to try to stuff ints into longs or longs into ints -- we can instead just choose the one that fits the task. Moreover: by having two functions and two type specifiers (one for ints and one for longs), the situation would almost be self-documenting: developers that would usually neglect to consider the difference between int and long would at least see that there is a 'int' variant and a 'long' variant and therefore be prompted -- without extensive re-education -- to consider using the correct 'hole' for the 'peg'. If Zend is to settle on everything being longs or everything being ints, then people have to be stopped from reintroducing the 'wrong' type and people are going to have problems when the 'other' storage is the only one that appears to fit the job. Given the gradual divergence of Zend PHP from 64-bit cleanliness, I had thought these things to be fairly urgent and fundamental. In any case, the number of users and lines of code influenced by these issues has become increasingly visible in public over the last four months. -- PHP Development Mailing List http://www.php.net/ To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [PATCH] - fix for 64 bit issues with OnUpdateInt
Hi, The reason the zend_parse_parameters() API only exposes long is because the Zend zval (container which holds the engine values) only supports longs and not ints. So anything which touches scripting engine values should be using long. However, extensions which have INI parameters may want to use ints for their internal logic. The INI directive retrieval really doesn't have anything to do with zval's. So I think the fix of adding OnUpdateLong() is the correct fix. We could add some support in ZEND_DEBUG mode to catch some common errors. For example, we could make the zend_parse_parameters() function a macro and make sure that people are passing long's and not ints. I'm not really sure it's worth it though. Andi At 06:19 PM 2/28/2003 +0800, James Devenish wrote: Hi, Preface: This e-mail uses the distribution list (To and CC addresses) that the original poster used. I have moved everyone other than php-dev to Bcc so that this doesn't get perpetuated (I have no idea of the significance of the other addresses that were e-mailed, nor do I know Dave Hill's standing in the PHP community). In message [EMAIL PROTECTED] on Thu, Feb 27, 2003 at 02:52:28PM -0500, Dave Hill wrote: Proposed Patch to address 64 bit issues in PHP v4.3.1 Diff -u against php4-STABLE-200302241430 Also needed in 4.5.x and 5.x Addresses bugs 20994, 21002, 21822, 20268 There was some discussion on php-dev in November 2002 after I posted patches for bug #20268 (the earliest-numbered one in Dave's list). I chose to address this problem by creating OnUpdateLong (in addition to to existing OnUpdateInt) and sweep though the code changing the call where needed to match the stucture item. An equally valid fix would be to change all of those longs to ints. (a) I also chose to create OnUpdateLong (and this has been working well for me since 4.3.0pre2 using the patches I posted). Since I posted those patches, PHP has progressed and more of these problems have been introduced (bad) but at the same time some of the test cases have been revised to be more portable (good). (b) I found that sometimes ints needed to stay as ints and longs needed to stay as longs, which is why I chose to separate OnUpdateLong from OnUpdateInt. (c) In discussions on php-dev in November, the subject of the thread was 64-bit PHP 4.3 (extensive long vs int problems) http://marc.theaimsgroup.com/?t=10369029101r=1w=2 (d) Jason Greene responded as a person who was working on this problem. Disfavour of the OnUpdateLong solution was expressed (on- and maybe off-list, too). After performing that sweep, I found there was a small number of other errors of the int/long type that my compiler found that I corrected and are included here. There might be more mix and match problems, I only addressed the ones in the modules I have enabled. Deja vu! Now the reason I am responding and the reason I have BCCed the people in Dave's list was that I wanted to mention something about Zend. Really, this bug has its roots in Zend though it is manifested in PHP modules. Firstly, OnUpdateInt assigns to long storage but uses zend_atoi -- clearly there is confusion already. But the more disturbing thing that I found in November was the Zend documentation for zend_parse_parameters (a site of extensive long/int problems). That is why I have written this e-mail. I know nothing about the Zend engine but I will refer to this document: http://www.zend.com/apidoc/zend.arguments.retrieval.php It is entitled Accepting Arguments / Retrieving Arguments and describes the zend_parse_parameters function. It lists a type specifier: l - long But it does not list an int type. In the actual code, it is entirely long-based, not int-based, so there is clearly a mismatch. But moreover, there is no acknowledgement of the significance of int versus long. So it is not surprising that so many developers have made mistakes arising from this. Therefore, I thought that a proper solution to this problem would have to involve some consideration of the Zend API (or at least copious acknowledgments and warnings in its documentation). There is potentially a huge developer education problem, too. That is why I chose to add OnUpdateLong. And given the choice, I would have 'i - int' added to the type specifiers of zend_parse_parameters. Firstly, having the OnUpdateLong function allows quick rectification of these problems without having to try to stuff ints into longs or longs into ints -- we can instead just choose the one that fits the task. Moreover: by having two functions and two type specifiers (one for ints and one for longs), the situation would almost be self-documenting: developers that would usually neglect to consider the difference between int and long would at least see that there is a 'int' variant and a 'long' variant and therefore be prompted -- without extensive re-education -- to consider using the correct 'hole' for the 'peg'. If Zend is to settle on everything being longs or everything being
Re: [PHP-DEV] [PATCH] - fix for 64 bit issues with OnUpdateInt
So I think the fix of adding OnUpdateLong() is the correct fix. I was under the impression that OnUpdateInt was actually expecting a long. I remember changing some int's to long's to address 64 bit issues. Do I remember this incorrectly? - Sascha -- PHP Development Mailing List http://www.php.net/ To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [PATCH] - fix for 64 bit issues with OnUpdateInt
At 04:28 PM 2/28/2003 +0100, Sascha Schumann wrote: So I think the fix of adding OnUpdateLong() is the correct fix. I was under the impression that OnUpdateInt was actually expecting a long. I remember changing some int's to long's to address 64 bit issues. Do I remember this incorrectly? No, you are correct but misunderstood me. We should introduce OnUpdateLong() for ppl using longs and use OnUpdateInt() for ppl who want to use ints. Andi -- PHP Development Mailing List http://www.php.net/ To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [PATCH] - fix for 64 bit issues with OnUpdateInt
So I think the fix of adding OnUpdateLong() is the correct fix. I was under the impression that OnUpdateInt was actually expecting a long. I remember changing some int's to long's to address 64 bit issues. Do I remember this incorrectly? - Sascha Most, but not all of the calls to OnUpdateInt were long, and at least one of the mismatches was in main (three longs in a row followed by an int look at output_buffering in main.c). I personally don't care which way the team wants to fix this. My thought was the making the name say long was the best way to avoid silly errors in the future. If you want some consistency, then you could always retire OnUpdateInt and force them all to be long :-) It is not as if an extra few bytes would be noticed. Part of the problem is that many people still think an int and a long are interchangeable - and they actually are when the compiler knows what it is dealing with. The problem is when you start passing pointers around and casting things, and the compiler can't fix things up for you. This is of course compounded by the fact that on quite a few platforms they are the same size Dave Hill -- PHP Development Mailing List http://www.php.net/ To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [PATCH] - fix for 64 bit issues with OnUpdateInt
No, you are correct but misunderstood me. We should introduce OnUpdateLong() for ppl using longs and use OnUpdateInt() for ppl who want to use ints. Well, and how are you planning to mitigate the BC issues? Remember that not all extension code is under our direct control. - Sascha -- PHP Development Mailing List http://www.php.net/ To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [PATCH] - fix for 64 bit issues with OnUpdateInt
At 04:47 PM 2/28/2003 +0100, Sascha Schumann wrote: No, you are correct but misunderstood me. We should introduce OnUpdateLong() for ppl using longs and use OnUpdateInt() for ppl who want to use ints. Well, and how are you planning to mitigate the BC issues? Remember that not all extension code is under our direct control. I don't think there's much choice. Do you have a better idea? If so, please explain it. Andi -- PHP Development Mailing List http://www.php.net/ To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [PATCH] - fix for 64 bit issues with OnUpdateInt
I think that simply adding OnUpdateLong and deprecating OnUpdateInt is fine while retaining its current semantics. I just don't see any value in changing the meaning of OnUpdateInt; at least that's how I interpreted Andi's message. - Sascha -- PHP Development Mailing List http://www.php.net/ To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [PATCH] - fix for 64 bit issues with OnUpdateInt
At 04:50 PM 2/28/2003 +0100, Sascha Schumann wrote: I think that simply adding OnUpdateLong and deprecating OnUpdateInt is fine while retaining its current semantics. I just don't see any value in changing the meaning of OnUpdateInt; at least that's how I interpreted Andi's message. That's also an option but I think OnUpdateInt() is confusing and how do we stop ppl from using it in new extensions which who's commit messages aren't followed via php-cvs? Andi -- PHP Development Mailing List http://www.php.net/ To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [PATCH] - fix for 64 bit issues with OnUpdateInt
That's also an option but I think OnUpdateInt() is confusing and how do we stop ppl from using it in new extensions which who's commit messages aren't followed via php-cvs? I volunteer to set up a cron job which greps for OnUpdateInt :-) The problem with _changing_ the existing semantics is that programmers will not notice that they need to adapt types from long to int. That leaves 64 bit platforms with a new set of problems, because the upper half of the long won't be initialized. - Sascha -- PHP Development Mailing List http://www.php.net/ To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [PATCH] - fix for 64 bit issues with OnUpdateInt
On Fri, 28 Feb 2003, Andi Gutmans wrote: At 04:50 PM 2/28/2003 +0100, Sascha Schumann wrote: I think that simply adding OnUpdateLong and deprecating OnUpdateInt is fine while retaining its current semantics. I just don't see any value in changing the meaning of OnUpdateInt; at least that's how I interpreted Andi's message. That's also an option but I think OnUpdateInt() is confusing and how do we stop ppl from using it in new extensions which who's commit messages aren't followed via php-cvs? You can't really, but at least their extensions won't compile in future PHP versions and they will be forced to go and address the issue as opposed to continuing to propogate extensions that aren't 64-bit friendly. If we could force a compiler warning/error on a type mismatch to the OnUpdateInt/Long then that might be a solution too. -Rasmus -- PHP Development Mailing List http://www.php.net/ To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [PATCH] - fix for 64 bit issues with OnUpdateInt
At 17:50 28/02/2003, Sascha Schumann wrote: I think that simply adding OnUpdateLong and deprecating OnUpdateInt is fine while retaining its current semantics. +1 Zeev -- PHP Development Mailing List http://www.php.net/ To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] [PATCH] - fix for 64 bit issues with OnUpdateInt
Proposed Patch to address 64 bit issues in PHP v4.3.1
Diff -u against php4-STABLE-200302241430
Also needed in 4.5.x and 5.x
Addresses bugs 20994, 21002, 21822, 20268
Platforms affected, Tru64, Solaris 9, HP-UX, NetBSD/Alpha and
any other 64 bit platform I forgot to mention
A little education first
on some platforms, sizeof(int) != sizeof(long)
This is particularly true on 64 bit platforms.
Mixed int* and long* in these cases can be deadly, and
can cause hard to find problems. This is because if
the machine writes to a long* that is really pointing at
an int, then extra data gets written past the end of the
int corrupting whatever is after that int.
Also on 64 bit platforms, only a long is going to be big enough
to hold a pointer, so assigning a pointer value to an int, and then
assigning it back will result in truncation of data.
Lastly, sizeof(size_t) == sizeof(long) which means if you plan to mix
things, don't mix size_t and int.
The particular problem that sparked this patch is a particular problem
with OnUpdateInt called from zlib, as this function assumes a long is
passed in,
despite the name. In the source code - this function was passed a long
in about 70% of the cases, an int in rest. What was passed was not
always consitant in a particular source module.
I chose to address this problem by creating OnUpdateLong (in addition to
to existing OnUpdateInt) and sweep though the code changing the call
where needed to match the stucture item. An equally valid fix would be
to change all of those longs to ints.
After performing that sweep, I found there was a small number of other
errors of the int/long type that my compiler found that I corrected and
are included here. There might be more mix and match problems, I only
addressed the ones in the modules I have enabled.
Files affected:
Zend/zend_ini.c
Zend/zend_ini.h
ext/fbsql/php_fbsql.c
ext/hyperwave/hw.c
ext/ingres_ii/ii.c
ext/interbase/interbase.c
ext/ldap/ldap.c
ext/mbstring/mbstring.c
ext/mssql/php_mssql.c
ext/mysql/php_mysql.c
ext/odbc/php_odbc.c
ext/pgsql/pgsql.c
ext/session/session.c
ext/standard/assert.c
ext/standard/file.c
ext/standard/info.c
ext/standard/url_scanner_ex.c
ext/sybase_ct/php_sybase_ct.c
main/main.c
sapi/apache/php_apache.c
--- php4-STABLE-200302241430/Zend/zend_ini.c2002-12-31 12:15:18.0 -0500
+++ php4-STABLE-200302241430.mod/Zend/zend_ini.c2003-02-27 14:25:11.0
-0500
@@ -427,9 +427,35 @@
return SUCCESS;
}
-
+/*
+ * Note: on 64 bit platforms, sizeof(long) != sizeof(int) and
+ * long* and int* cannot be used interchangably without disaster !
+ * OnUpdateInt or OnUpdateLong must match the resource type !
+ */
ZEND_API ZEND_INI_MH(OnUpdateInt)
{
+ int *p;
+#ifndef ZTS
+ char *base = (char *) mh_arg2;
+#else
+ char *base;
+
+ base = (char *) ts_resource(*((int *) mh_arg2));
+#endif
+
+ p = (int *) (base+(size_t) mh_arg1);
+
+ *p = zend_atoi(new_value, new_value_length);
+ return SUCCESS;
+}
+
+/*
+ * Note: on 64 bit platforms, sizeof(long) != sizeof(int) and
+ * long* and int* cannot be used interchangably without disaster !
+ * OnUpdateInt or OnUpdateLong must match the resource type !
+ */
+ZEND_API ZEND_INI_MH(OnUpdateLong)
+{
long *p;
#ifndef ZTS
char *base = (char *) mh_arg2;
@@ -445,7 +471,6 @@
return SUCCESS;
}
-
ZEND_API ZEND_INI_MH(OnUpdateReal)
{
double *p;
--- php4-STABLE-200302241430/Zend/zend_ini.h2002-12-31 12:15:18.0 -0500
+++ php4-STABLE-200302241430.mod/Zend/zend_ini.h2003-02-27 14:25:11.0
-0500
@@ -171,6 +171,7 @@
/* Standard message handlers */
ZEND_API ZEND_INI_MH(OnUpdateBool);
ZEND_API ZEND_INI_MH(OnUpdateInt);
+ZEND_API ZEND_INI_MH(OnUpdateLong);
ZEND_API ZEND_INI_MH(OnUpdateReal);
ZEND_API ZEND_INI_MH(OnUpdateString);
ZEND_API ZEND_INI_MH(OnUpdateStringUnempty);
--- php4-STABLE-200302241430/ext/fbsql/php_fbsql.c 2003-02-12 16:13:08.0
-0500
+++ php4-STABLE-200302241430.mod/ext/fbsql/php_fbsql.c 2003-02-27 14:25:11.0
-0500
@@ -358,14 +358,14 @@
/* {{{ PHP_INI
*/
PHP_INI_BEGIN()
- STD_PHP_INI_BOOLEAN (fbsql.allow_persistent, 1,
PHP_INI_SYSTEM, OnUpdateInt,allowPersistent, zend_fbsql_globals,
fbsql_globals)
- STD_PHP_INI_BOOLEAN (fbsql.generate_warnings,0,
PHP_INI_SYSTEM, OnUpdateInt,generateWarnings, zend_fbsql_globals,
fbsql_globals)
- STD_PHP_INI_BOOLEAN (fbsql.autocommit,
1,PHP_INI_SYSTEM, OnUpdateInt,autoCommit,
zend_fbsql_globals, fbsql_globals)
- STD_PHP_INI_ENTRY_EX (fbsql.max_persistent, -1,
PHP_INI_SYSTEM, OnUpdateInt,maxPersistent,zend_fbsql_globals,
fbsql_globals, display_link_numbers)
- STD_PHP_INI_ENTRY_EX (fbsql.max_links,
128,
