Hooks passing by ref about smf HOT 18 CLOSED

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...

from smf.

It's deprecated in PHP 5.3 as well.

from smf.

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

from smf.

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

from smf.

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

from smf.

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).

from smf.

@emanuele45 , does that commit make this closed?

from smf.

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.

from smf.

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

from smf.

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

from smf.

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).

from smf.

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.

from smf.

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.

from smf.

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.

from smf.

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.)

from smf.

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

from smf.

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...

from smf.

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.

from smf.