Hooks passing by ref

[live627]

Before PHP 5.4, referenced variables were passed to the function by reference, regardless of whether the function expects the respective parameter to be passed by reference. This form of call-time pass by reference did not emit a deprecation notice, even though it was deprecated, and has been removed in PHP 5.4.

So I suggest all variables passed through call_integration_hook are done by value. Removing the ampersand in front of the var.

Lastly, I'll note that some poorly written plugins will break. But they already break in 5.4, so this argument is invalid.

[emanuele45]

Yes, that's true.
Though the availability of php 5.4 doesn't seem to be so high.

What I proposed a while ago (don't remember where), was something intermediated: all the new hooks remove the ampersand, for the old ones keep it for a bit of compatibility.
In case php 5.4 becomes very used in a short period we can apply a patch later on and fix everything.

Though I don't have big problems with one or the other solution...

[joshuaadickerson]

It's deprecated in PHP 5.3 as well.

[live627]

Yes, but does not give a w warning if passed through call_user_func_array().

[joshuaadickerson]

What I am getting at is that we should be logging that as a bug and changing the hooks before we release 2.1.

[live627]

I totally agree! In fact, that is precisely the reason I opened this issue. Now, to get the dev heads to agree....

[Oldiesmann]

I think we should definitely change this for 2.1, and also post something on SMF alerting mod authors to this issue so they can fix their code. I ran into this issue with my SMF+G2 mod when I upgraded to PHP 5.4 a while back, and I'm sure others will as well (though I haven't seen any support posts relating to that issue on the support forum for that mod).

[joshuaadickerson]

@emanuele45 , does that commit make this closed?

[emanuele45]

Not so sure...it's a step.
Another one could be break things in the beta (i.e. remove all the pass-by-ref) and see what will happen and then decide what to do for the final based on the feedback.

[norv]

Num: #209 doesn't change pass by ref. What is the idea right now, pass by value for new hooks only?

[Spuds]

I believe the current state is that all new hooks added in 2.1 only pass by value so they are 5.4 compliant. There is still discussion if we want to make a breaking commit and change the ones that were in place at 2.0. At some point those will have to be changed regardless so seems like a better to do sooner than later ... IMO of course

[norv]

I agree at this point. The changes will need documented, preferably both on the development wiki as well as a blog dedicated to hooks in 2.1. Explanation, use example(s).

[Oldiesmann]

How many integration hooks are still called by reference like this?

PHP 5.3 is essentially EOL now (5.3.27 is the last release in that line apparently) and PHP 5.5.0 went gold last month, so we should start planning to make SMF compatible with 5.5.

[emanuele45]

The point of this error is not the call-by-ref, the error in all the hooks that were throwing that error was that the parameters were not in an array:
'''
call_integration_hook('integrate_something', &$context['something']);
'''
Instead it should be:
'''
call_integration_hook('integrate_something', array(&$context['something']));
'''

And with the latest revert I did 5 months ago SMF 2.1 should already compatible with php 5.4 and 5.5.

[Oldiesmann]

Thanks for the insight. I'll go through the hooks in the coming days and fix any remaining issues along those lines.

I haven't tested PHP 5.5 at all, so I don't know what, if anything, needs to be done for that.

[Arantor]

Does anything else need to be done with this?

(Fairly sure, incidentally, that the values inside the arrays actually have to be references if the receiving function is also expecting references. This might mean we should be checking what we want to make available and check that things are so. Perhaps we need a test suite for all hooks to validate that sensible values are provided for each hook.)

[Oldiesmann]

I never got around to fixing the remaining hooks apparently, so that still needs to be done.

[Arantor]

What does? I looked through the hooks quickly, I couldn't find any hooks that weren't passing their exported values with arrays as they're supposed to...

[Arantor]

OK, I'm going to mark this closed because I'm not sure what actually needs to be done. If there are any problems with any existing hooks, we'll deal with them as we find them for now.