zend_hash_sort always returns SUCCESS

[rjhdby]

zend_hash_sort (zend_hash_sort_ex) never returns FAILURE

[KalleZ]

Hmm, if it always returns true then perhaps we should simply change it to return void? However if its ZEND_API then most likely only master/PHP-7.4 only

[rjhdby]

@KalleZ Some extensions may use this return value

[KalleZ]

@rjhdby well yes, and if they do, then they should be encouraged to change it, hence why patching it in 7.4 or 8.0 makes sense to keep ABI, migration path is rather easy and still cross version compatible (as your patch already displays)

[rjhdby]

@KalleZ done

[cmb69]

If we're going to change the return type of zend_hash_sort_ex(), we should change the return type of zend_ts_hash_sort(), too.

[cmb69]

LGTM. Thanks!

[nikic]

Merged as e86cdce.

[Jan-E]

Do you all imply that a check like Solr does in https://github.com/php/pecl-search_engine-solr/blob/master/src/php7/php_solr_input_document.c#L658 is senseless?

	if (zend_hash_sort_ex(doc_entry->fields, zend_qsort, comparison_function, renumber TSRMLS_CC) == FAILURE) {

		RETURN_FALSE;
	}

	RETURN_TRUE;

Would this never return FALSE?

And/or Is the change in 33ef3d6 related?

[Jan-E]

And shouldn't this change be documented in UPGRADING.INTERNALS?

[cmb69]

@Jan-E the only reason zend_hash_sort() could ever return FAILURE was due to failure to allocate heap memory; since the ZendMM is infallible, this can no longer happen (as of PHP 5.5.0). However, since the return type is int before PHP 8, checking for FAILURE/SUCCESS makes sense; it no longer makes sense for PHP 8 (and actually triggers a compile error).

Anyway, the signature change is now documented: 942f341.

[Jan-E]

OK, so the new PHP8 (and even PHP7) code for Solr should be

	zend_hash_sort_ex(doc_entry->fields, zend_qsort, comparison_function, renumber TSRMLS_CC);

	RETURN_TRUE;

And a comparable change for ereg (yeah, I know):
http://git.php.net/?p=pecl/text/ereg.git;a=blob;f=ereg.c;h=5768a68344104715393627788f01147d32a41874;hb=refs/heads/master#l143

[cmb69]

yes :)

[Jan-E]

zend_hash_sort and zend_hash_sort compare function signature change

Did you mean to stress the change ;)

[cmb69]

Well, both the signature of zend_hash_function, and the signature of the zend_hash_sort compare function has been changed. Not sure how to express that better.

[Jan-E]

Or should it be read as

zend_hash_sort compare function and zend_hash_sort signature change