Conversation
Set undefined values to null rather than undefined.
cmb69
left a comment
cmb69
left a comment
There was a problem hiding this comment.
Thanks! The patch looks good to me, but there appear to be some issues in the PHPT.
ext/libxml/tests/bug76777.phpt
Outdated
| --TEST-- | ||
| Bug #76777 (first parameter of libxml_set_external_entity_loader callback undefined) | ||
| --SKIPIF-- | ||
| <?php if (!extension_loaded('libxml')) die('skip'); ?> |
There was a problem hiding this comment.
The test should also be skipped if --offline is given, i.e. something like if (getenv("SKIP_ONLINE_TESTS")) die('skip online test'); would be reasonable.
ext/libxml/tests/bug76777.phpt
Outdated
| <?php if (!extension_loaded('libxml')) die('skip'); ?> | ||
| --FILE-- | ||
| <?php | ||
| ini_set('error_reporting',PHP_INT_MAX-1); |
There was a problem hiding this comment.
Usually, we do not set error_reporting unless there's a particular reason to do so, which doesn't seem to be the case here (also PHP_INT_MAX-1 is quite uncommon to my knowledge).
ext/libxml/tests/bug76777.phpt
Outdated
|
|
||
| libxml_set_external_entity_loader(function($p,$s,$c) { | ||
| var_dump($p,$s,$c); | ||
| die(); |
There was a problem hiding this comment.
To die here seems doubtful. I'd rather remove the var_dump 4 lines below.
There was a problem hiding this comment.
die is there to avoid schema parsing errors on output as they are not relevant to the test, but are artifact of simplistic test case:
Warning: DOMDocument::schemaValidateSource(): Element '{http://www.w3.org/2001/XMLSchema}include': Failed to parse the XML resource 'nonexistent.xsd'. in /in/Tt5TM on line 20
Warning: DOMDocument::schemaValidateSource(): Invalid Schema in /in/Tt5TM on line 20
ext/libxml/tests/bug76777.phpt
Outdated
| libxml_set_external_entity_loader(function($p,$s,$c) { | ||
| var_dump($p,$s,$c); | ||
| die(); | ||
| }); |
|
Comment on behalf of cmb at php.net: Thanks. Applied via cf2fc66. |
3 participants
Set undefined values to null rather than undefined.
Fixes bug #76777.