Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Changed: Implementing improved error callback mechanism (php_error_cb) #1247

Conversation

patrickallaert
Copy link
Contributor

Olivier and others added 3 commits April 21, 2015 18:34
Fixes "error: ‘core_globals_id’ undeclared" when PHP is compiled with
--enable-maintainer-zts
Error hooks should normally return SUCCESS but they can return FAILURE
in order to stop the processing of the current hook list as it is the
case for the bailout one.

It also has the benefit of standardizing the API for error hooks.
@Kubo2
Copy link
Contributor

Kubo2 commented Apr 26, 2015

👍

char *log_buffer;
#ifdef PHP_WIN32
if (type == E_CORE_ERROR || type == E_CORE_WARNING) {
syslog(LOG_ALERT, "PHP %s: %s (%s)", error_type_str, buffer, GetCommandLine());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs win32/syslog.h to be included. Or maybe it'd be better to check what header inclusion did provide it in the old file. For the rest it looks fine.

Thank you Anatol Belski for noticing.
@patrickallaert
Copy link
Contributor Author

@weltling Should be fixed in 13183ba, thanks for noticing Anatol!

@@ -959,12 +960,26 @@ PHPAPI void php_html_puts(const char *str, size_t size)
}
/* }}} */

#define CALL_ERROR_HOOKS(HOOK_LIST) for ( \
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not make it inline function instead of macro? Would be much more readable.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have unpleasant experience with va_list and I guess I would have to va_copy/va_end once more if I am using a function rather than a macro, but more important than that, gcc has/had issues with va_list being passed to an inline function. Additionally, I guess that it wouldn't be inlined at all in the end, as it's just a hint for the compiler.

@smalyshev smalyshev added the RFC label May 8, 2015
@krakjoe
Copy link
Member

krakjoe commented Jan 2, 2017

@patrickallaert fix conflicts please :)

Also take this as a reminder to push forward with the RFC ... if you consider this work abandoned please close this PR.

@krakjoe
Copy link
Member

krakjoe commented Feb 3, 2017

Having waited more than a month for this PR to be fixed, and since there are still merge conflicts, I'm closing this PR, and will update the RFC.

Please take this action as encouragement to open a clean PR against a supported branch, and push forward with the RFC.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants