Allow resetting the error handler by nikic · Pull Request #18 · php/php-src

nikic · 2012-03-23T21:11:33Z

This allows the error handler to be reset using set_error_handler(null). As the code suggests this behavior was already previously intended, but the callback check was done too strictly.

See also bug https://bugs.php.net/bug.php?id=60738.

PS: I replaced the !zend_is_true with a strict null check to be consistent with the behavior of set_exception_handler.

This allows the error handler to be reset using set_error_handler(null). As the code suggests this behavior was already previously intended, but the callback check was done too strictly.

laruence · 2012-03-24T03:27:09Z

Hi, I attached another fix, this will save one alloc/free pair, and also save one (IS_NULL == Z_TYPE_P(error_handler).
here it is:https://bugs.php.net/patch-display.php?bug_id=60738&patch=bug60738.patch&revision=latest

Nikic, if you have no objection, I will commit the patch. and later, also improve the set_exception_handler.

thanks

laruence · 2012-03-24T07:19:32Z

since I will offline for maybe two days, I commited, if you have any question, plz write me :) thanks

php-pulls · 2012-03-24T07:20:43Z

Comment on behalf of laruence at php.net:

implemented

php-pulls · 2012-03-24T07:22:21Z

Comment on behalf of laruence at php.net:

seems I can not close this pull request via qa.php.net

nikic · 2012-03-24T08:21:01Z

@laruence I wanted to submit another PR which makes set_error_handler(null) return the previous error handler instead of just true. That's why I left the code intact, as it was easier to implement with the old code structure.

smalyshev · 2012-03-24T08:48:58Z

set_error_handler(null) should return previous one just like all others. If it's not so with new patch, should be fixed.

laruence · 2012-03-24T09:42:58Z

On Sat, Mar 24, 2012 at 4:48 PM, Stanislav Malyshev
reply@reply.github.com
wrote:

set_error_handler(null) should return previous one just like all others. If it's not so with new patch, should be fixed.
Hi:
in the manual of set_exception_handler says:
" Returns the name of the previously defined exception handler, or
NULL on error. If no previous handler was defined, NULL is also
returned. If NULL is passed, resetting the handler to its default
state, TRUE is returned. "

thanks

Reply to this email directly or view it on GitHub:
#18 (comment)

惠新宸 laruence
Senior PHP Engineer
http://www.laruence.com

laruence · 2012-03-24T09:44:03Z

On Sat, Mar 24, 2012 at 4:21 PM, nikic
reply@reply.github.com
wrote:

@laruence I wanted to submit another PR which makes set_error_handler(null) return the previous error handler instead of just true. That's why I left the code intact, as it was easier to implement with the old code structure.

I don't think that should be done, since set_exception_handler return
TRUE when passing NULL to it.

thanks

Reply to this email directly or view it on GitHub:
#18 (comment)

惠新宸 laruence
Senior PHP Engineer
http://www.laruence.com

laruence · 2012-03-24T09:45:05Z

On Sat, Mar 24, 2012 at 4:21 PM, nikic
reply@reply.github.com
wrote:

@laruence I wanted to submit another PR which makes set_error_handler(null) return the previous error handler instead of just true. That's why I left the code intact, as it was easier to implement with the old code structure.

And, I really don't think your new PR should relay on a code
structure, so, just file your new FR. :)

thanks

Reply to this email directly or view it on GitHub:
#18 (comment)

惠新宸 laruence
Senior PHP Engineer
http://www.laruence.com

smalyshev · 2012-03-24T09:50:23Z

I have no idea why set_exception_handler() returns true in this case, makes very little sense.

php-pulls · 2012-03-24T09:54:21Z

On Sat, Mar 24, 2012 at 5:50 PM, Stanislav Malyshev
reply@reply.github.com
wrote:

I have no idea why set_exception_handler() returns true in this case, makes very little sense.
So, that is another issue.

In this fix, I made them consistent.

if we decide to change the return value, then set_exception_handler
should also be change.

thanks

Reply to this email directly or view it on GitHub:
#18 (comment)

Git Pull Requests Mailing List (https://github.com/php)
To unsubscribe, visit: http://www.php.net/unsub.php

Laruence Xinchen Hui
http://www.laruence.com/

smalyshev · 2012-03-24T10:02:45Z

This is why changes like this should be done in a branch, announced on the list and discussed, not put into stable branch first and discussed later.

laruence · 2012-03-24T10:08:10Z

On Sat, Mar 24, 2012 at 6:02 PM, Stanislav Malyshev
reply@reply.github.com
wrote:

This is why changes like this should be done in a branch, announced on the list and discussed, not put into stable branch first and discussed later.

what change? you mean "allow NULL value pass to set_error_handler"?

I think this is a little change, so I committed it, but yeah, if
there comes objections, I can revert it. :)

thanks

Reply to this email directly or view it on GitHub:
#18 (comment)

惠新宸 laruence
Senior PHP Engineer
http://www.laruence.com

smalyshev · 2012-03-24T10:11:07Z

first, it's not a little change, it changes a lot in this function. Second, I'm not sure these functions should work this way, and I see no reason to rush with it without discussion. Please revert it for from 5.4, put it into a feature branch and let's see what's the best way to take there.

laruence · 2012-03-24T10:14:09Z

On Sat, Mar 24, 2012 at 6:11 PM, Stanislav Malyshev
reply@reply.github.com
wrote:

first, it's not a little change, it changes a lot in this function. Second, I'm not sure these functions should work this way, and I see no reason to rush with it without discussion. Please revert it for from 5.4, put it into a feature branch and let's see what's the best way to take there.

okey, how about the previous one: "improve set_exception_handler",
also revert it? it change code but not behavior..

thanks

Reply to this email directly or view it on GitHub:
#18 (comment)

惠新宸 laruence
Senior PHP Engineer
http://www.laruence.com

nikic · 2012-03-24T11:43:32Z

Sorry for causing all this confusion. I'll create a new PR which will include all changes including the fix for the return value and the memory leak which currently exists in these functions.

php-pulls · 2012-03-24T11:44:26Z

Hi:

Reverted. both the two changes, then committed to trunk only .

thanks

On Sat, Mar 24, 2012 at 6:14 PM, Xinchen Hui
reply@reply.github.com
wrote:

On Sat, Mar 24, 2012 at 6:11 PM, Stanislav Malyshev
reply@reply.github.com
wrote:

first, it's not a little change, it changes a lot in this function. Second, I'm not sure these functions should work this way, and I see no reason to rush with it without discussion. Please revert it for from 5.4, put it into a feature branch and let's see what's the best way to take there.

okey, how about the previous one: "improve set_exception_handler",
also revert it? it change code but not behavior..

thanks

Reply to this email directly or view it on GitHub:
#18 (comment)

惠新宸 laruence
Senior PHP Engineer
http://www.laruence.com

Reply to this email directly or view it on GitHub:
#18 (comment)

Git Pull Requests Mailing List (https://github.com/php)
To unsubscribe, visit: http://www.php.net/unsub.php

Laruence Xinchen Hui
http://www.laruence.com/

laruence · 2012-03-24T11:51:42Z

Nikic:

np, it's alright, we always doing commit/revert.. :)

thanks

On Sat, Mar 24, 2012 at 7:43 PM, nikic
reply@reply.github.com
wrote:

Sorry for causing all this confusion. I'll create a new PR which will include all changes including the fix for the return value and the memory leak which currently exists in these functions.

Reply to this email directly or view it on GitHub:
#18 (comment)

惠新宸 laruence
Senior PHP Engineer
http://www.laruence.com

Errors not displaying in some configurations

* PHP-5.6: fixed incompatible pointer in phpdbg on win64 Fixed php#18 Errors not displaying in some configurations Fixed php#18 Errors not displaying in some configurations update credits headers credits file for phpdbg

Allow resetting the error handler

350fd7b

This allows the error handler to be reset using set_error_handler(null). As the code suggests this behavior was already previously intended, but the callback check was done too strictly.

php-pulls closed this Mar 24, 2012

nikic mentioned this pull request Mar 24, 2012

Improve error and exception handlers #20

Closed

php-pulls pushed a commit that referenced this pull request Dec 24, 2013

Fixed #18

8064d8d

Errors not displaying in some configurations

php-pulls pushed a commit that referenced this pull request Dec 24, 2013

Fixed #18

ddf733f

Errors not displaying in some configurations

php-pulls pushed a commit that referenced this pull request Oct 28, 2014

Fixed #18

36f487a

Errors not displaying in some configurations

dstogov referenced this pull request Apr 17, 2015

Added GC checks and improvements

276080e

0xhacking mentioned this pull request Mar 17, 2022

Opcache enable Crash Core dump #8209

Closed

ffang-imvu mentioned this pull request Jun 15, 2022

Segmentation faults in 8.1.5 with opcache enabled #8797

Closed

pfrappe mentioned this pull request Jan 28, 2023

Apache Segmentation fault (11) disappears after print_r on a variable. can't understand... #10430

Open

dunglas mentioned this pull request Sep 17, 2023

Segmentation fault when building Symfony cache on Alpine #12234

Closed

Conversation

nikic commented Mar 23, 2012

Uh oh!

laruence commented Mar 24, 2012

Uh oh!

laruence commented Mar 24, 2012

Uh oh!

php-pulls commented Mar 24, 2012

Uh oh!

php-pulls commented Mar 24, 2012

Uh oh!

nikic commented Mar 24, 2012

Uh oh!

smalyshev commented Mar 24, 2012

Uh oh!

laruence commented Mar 24, 2012

Uh oh!

laruence commented Mar 24, 2012

Uh oh!

laruence commented Mar 24, 2012

Uh oh!

smalyshev commented Mar 24, 2012

Uh oh!

php-pulls commented Mar 24, 2012

Uh oh!

smalyshev commented Mar 24, 2012

Uh oh!

laruence commented Mar 24, 2012

Uh oh!

smalyshev commented Mar 24, 2012

Uh oh!

laruence commented Mar 24, 2012

Uh oh!

nikic commented Mar 24, 2012

Uh oh!

php-pulls commented Mar 24, 2012

Uh oh!

laruence commented Mar 24, 2012

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Comments