Skip to content

Remove --disable-segfault-error#285

Merged
kamil-tekiela merged 1 commit intophp:masterfrom
kamil-tekiela:disable-segfault-error
Apr 23, 2026
Merged

Remove --disable-segfault-error#285
kamil-tekiela merged 1 commit intophp:masterfrom
kamil-tekiela:disable-segfault-error

Conversation

@kamil-tekiela
Copy link
Copy Markdown
Member

I sure hope that 20 years later, the segfault has been fixed.

Copy link
Copy Markdown
Member

@ndossche ndossche left a comment

Choose a reason for hiding this comment

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

Yeah that makes sense to do. Also I don't see how libxml_use_internal_errors ever actually made a difference in this regard.

@kamil-tekiela kamil-tekiela merged commit 0eb3611 into php:master Apr 23, 2026
11 of 12 checks passed
@alfsb
Copy link
Copy Markdown
Member

alfsb commented Apr 23, 2026

I sure hope that 20 years later, the segfault has been fixed.

Hun... the only unset() that exists in configure.php was added to fix a segfault as recent as 2025-03.

Yeah that makes sense to do. Also I don't see how libxml_use_internal_errors ever actually made a difference in this regard.

Maybe it's something with the newer "call stack" output. In fact, the example in manual for libxml_use_internal_errors() uses it before doing any foreach on libxml_get_errors().

@kamil-tekiela
Copy link
Copy Markdown
Member Author

Hun... the only unset() that exists in configure.php was added to fix a segfault as recent as 2025-03.

I don't think that's a good idea. A segfault needs to be fixed in php-src, and then when a user gets segfault, it means they are using an unpatched version of PHP. Hiding this will make the code usable immediately, but it won't fix the problem and creates code that will linger for years without anyone other than the author knowing why it is there in the first place. And when it gets fixed, even the author may forget why it was added.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants