PHP Strict Standards: Only variables should be passed by reference in wp-includes/class-oembed.php on line 139

[markjaquith]

Can't do this all in one line:

if ( $html = wp_remote_retrieve_body( wp_safe_remote_get( $url ) ) ) {

[ocean90]

There is a similar line in wp-includes/class-wp-xmlrpc-server.php.

[nacin]

Yeah, I had noticed these the other day. It appears the byref stuff here is from an older era and should be removed from all wp_remote_retrieve_* functions. I asked dd32 and he concurred.

[nacin]

Let's just make a decision one way or another here. I'm fine with just papering over the notice now if we think removing the byref late in a release cycle might have unintended or unpredictable side effects.

These functions never actually modify $response, but $response can be more than just an object. It's WP_Error on failure but otherwise it's a big array on success. (Replacing this with an object with ArrayAccess could be interesting, at some point.)

[dd32]

I am almost 100% certain that removing these [the byrefs] will have no detrimental impact. The usage of byref appears to be due to an attempt to keep memory footprint small and avoid the (potentially large) HTTP response being copied multiple times.. that being said, I'm not even sure if that was a risk under PHP4..

If the $response was an object instead of an array (..which it ideally should've been) then we may have had object copying memory-related issues in PHP4 but it wasn't, so there's really no reason why to use a byref here.

I'm on the fence still, a) removing the refs, b) splitting the oembed calls, c) leaving it as-is and removing the refs at the start of the next cycle.. lets face it, this isn't a new warning, although I don't like it as much as the rest of you.

[nacin]

I created #27687.

[nacin]

In 27957:

Don't pass variables by reference.

props markjaquith.
fixes #27656.