Standardize PHP's docref errors

[Girgias]

Converting all php_error_docref0(), php_error_docref1() and php_error_docref2() to use the standard php_error_docref() function to generate clickable errors.

Rationale:

The php_error_docref1 and php_error_docref2 functions aren't really used outside of streams, the DBA extension, and a couple of edge cases. The php_error_docref0 is redefined as php_error_docref so those conversions are straight forward.

Moreover, there is no more mention about the differences and use cases of the php_error_docref<n> functions in PHP 5 nor 7 coding standard contrary to what the source code says, and to get any information about them, one must look at a PHP 4 version to see what they are about. [1]

Going by this it seems that as of PHP 5 the recommended way of generating clickable errors with an optional docref is by using the php_error_docref macro which is bound to php_error_docref0

A second motivation is that all of PHP's errors would look the same. Currently there are some case, as said before mostly related to streams, where an error message looks like Error_Type: function(param): [...] if generated by using php_error_docref1() or Error_Type: function(param1, param2): [...] if generated by using php_error_docref2() which may be slightly confusing.

Improvements:

• All failed to open stream errors state the stream just after this text instead of in the function brackets.
• With rename() better description of which part is the problem, source path or target path.
• ext/standard/tests/strings/bug68996.phpt does now report the function i.e. fopen() as to where the error is caused.

Changes:

• php_win32_docref2_from_error() has been renamed to php_win32_docref_from_error_with_param as its behaviour changed, this however shouldn't affect much code as it was only used 4 times in streams/plain_wrapper.c. [3]

List of areas not covered by unit tests:

• Debug notices in EXIF extension.
• Multiple failure states in DBA extension.
• Some branches within streams:
  • _php_stream_open_wrapper_ex (streams.c) default case in switch statement
  • php_plain_files_metadata (plain_wrapper.c)
  • php_plain_files_rename (plain_wrapper.c) when EXDEF is defined: branch if (errno == EXDEV)

Possible bugs uncovered:

• opendir() [3] emit a Warning on Windows even though the message says the operation was completed successfully.

References

[1] https://github.com/php/php-src/blob/PHP-4.0/CODING_STANDARDS
[2] https://lxr.room11.org/search?project=php-src%407.4&q=php_win32_docref2_from_error&defs=&refs=&path=&hist=&type=
[3] See test: ext/standard/tests/dir/opendir_variation6-win32.phpt

[Girgias]

I'm wondering if some of the php_error in extension code into php_error_docref as it seems not as widely used c.f. [1], however, haven't really checked how the behavior differs as it is part of the Zend API and not the PHP API.
[1] https://github.com/php/php-src/search?q=php_error&unscoped_q=php_error

[nikic]

I'm not sure I really understand what the motivation for this change is. Can you maybe explain in more detail what the advantage of migrating everything to docref0 is?

[Girgias]

I'm not sure I really understand what the motivation for this change is. Can you maybe explain in more detail what the advantage of migrating everything to docref0 is?

Ah yes seems like an important point, main reason is that outside of streams (and DBA extensions, and 2 other cases IIRC) the docref1&2 functions aren't used, moreover there is no more mention about the differences of these different docrefs in PHP 5 nor 7 coding standard and I needed to look at a PHP 4 version to see what they are about [1] so it seems that as of PHP 5 the recommended way of displaying docref errors is by using the php_error_docref macro which is bound to docref0

The second motivation is that all errors would look the same instead of having some cases where you can get function(param): error message in case of docref1 and function(param1, param2): error message in case of docref2 which may be slightly confusing IMHO.

This, in turn, could mean that we merge together php_error_docref0 and php_verror (after converting the custom error handlers of GD, EXIF and SNMP extension [2] to use docref0) to only have one way how extensions display errors to users. Scratch after some sleep this is pretty dumb, however maybe renaming php_verror into something like php_error_docref_va_list to be more in line with other functions?

This last bit may be a bit overkill but it is an idea.

This does, however, impact extensions so if this kind of change is better for PHP 8 I'm fine with that too.

[1] https://github.com/php/php-src/blob/PHP-4.0/CODING_STANDARDS
[2] https://github.com/php/php-src/search?q=php_verror

[Girgias]

It looks like I uncovered a genuine bug, opendir() seems to throw a warning on Windows even if it succeeds.

[Girgias]

Would it be better that I leave the php_error_docref1() and php_error_docref2() functions for this to land or are the more issues with this?