Remove & reference declaration from db_query to avoid warning

[onli]

Also adjusts the code that calls the function with a prepended &, as that would have caused a new warning. Fixes #848.

[onli]

@garvinhicking I don't currently understand the idea of the & function declaration. What was the idea here, how could database query results be passed as reference and thus mutated? I assume I'm missing something, this is not my area of expertise.

[onli]

Okay, I looked into this a bit more. My research so far:

https://www.php.net/manual/en/language.references.return.php shows clearly how this is supposed to work (now). A class variable is shared by returning it as a reference. This example does not apply here, as serendipity_db_query returned with $rows a variable the function itself created, so the alias we are creating here does not point to something shared.

Why is the code like this? I do not know, but it is at least 19 years old,

Serendipity/include/db/mysql.inc.php

Line 61 in 80e1434

function &serendipity_db_query($sql, $single = false, $result_type = "both", $reportErr = false, $assocKey = false, $assocVal = false, $expectError = false) {

already returned the reference. My guess is that this was a performance optimization aimed at PHP 4 - it likely reduced memory usage back then by not having to copy the function return value to a new variable.

Will this change have a negative impact? According to my code search in additional_plugins, almost all calls to serendipity_db_query already use the regular assignment ($somevar = serendipity_db_query(...)). Exception is serendipity_event_userprofile, which uses at https://github.com/s9y/additional_plugins/blob/ab6addd0f8f708b09b651a365dc05641ac8c7134/serendipity_event_userprofiles/serendipity_event_userprofiles.php#L276 a reference assignment, but looking at the following code I see no purpose to that. Similar to how showUser(&$user) in the plugin passes the argument by reference without doing anything with it later but reading it, which means it does so needlessly (in the current iteration of the code).

Second usage is that https://github.com/s9y/additional_plugins/blob/master/emerge_spartacus.php#L22 copies the function declaration, so this would have to be changed later when spartacus is build with a new s9y version.
It is possible I missed a function call here though, and ofc plugins outside of spartacus might be unknowingly affected.

---

I intend to merge this. It silences a notice, should make the code easier and I do not expect bugs, as the unclear semantics of the reference passing probably made it meaningless in PHP >= 5. I'll give it a few more days for potential feedback though :)

[garvinhicking]

Thanks for taking care.

Yes, this was a micro optimization to hint PHP at "knowing" it shall not re-create intermediate variables in memory for passing function results around, so no "copy by variable".

PHP8 is so much beyond needing such an optimization so for legibility, this change should be totally fine I believe.