Make WordPress Core

Opened 13 years ago

Closed 12 years ago

#20756 closed defect (bug) (fixed)

Wrong reference returned in function &get_pages

Reported by: thomask's profile thomask Owned by: foxinni's profile foxinni
Milestone: 3.5 Priority: normal
Severity: trivial Version: 3.3.2
Component: Warnings/Notices Keywords: has-patch
Focuses: Cc:

Description

wp-includes/post.php line 3326

function &get_pages($args = '') {

should be

function get_pages($args = '') {

otherwise the

return false;

e.g. on line 3347 when requesting get_page with non-hierarchical post-type would throw something like

Notice: Only variable references should be returned by reference in /var/www/vhosts/money.cz/httpdocs/wp-includes/post.php on line 3347

(sorry i do not know how to make patches)

Attachments (3)

20756.patch (462 bytes) - added by SergeyBiryukov 13 years ago.
page-by-reference.diff (936 bytes) - added by wonderboymusic 12 years ago.
20756.2.patch (437 bytes) - added by Viper007Bond 12 years ago.
When using the cache, return a variable instead of the results of a function

Download all attachments as: .zip

Change History (14)

#1 @SergeyBiryukov
13 years ago

  • Component changed from General to Warnings/Notices
  • Keywords has-patch added; needs-patch removed

Introduced in [2478].

Related: #12821

#2 @foxinni
12 years ago

  • Keywords needs-patch added
  • Owner set to foxinni
  • Status changed from new to reviewing
  • Summary changed from wrong reference in function &get_pages to Wrong reference returned in function &get_pages
  • Version changed from 3.3.2 to 3.4.1

I can confirm this issue.

This only happens when it's used inside of a plugin, and on Multi-Site installs. (Needs confirming)

The error I get:

Notice: Only variable references should be returned by reference in /Users/admin/Sites/wp-includes/post.php on line 3394

Changing &get_pages to get_pages does resolve the issue...

Also returning an actual variable resolves the issue:

if ( !in_array( $post_type, $hierarchical_post_types ) ) {
      $rtn_variable = false;
      return $rtn_variable;
}

Can't be sure if any of these patched/fixes are at the root of the problem.

#3 @SergeyBiryukov
12 years ago

  • Keywords needs-patch removed
  • Milestone changed from Awaiting Review to 3.5
  • Version changed from 3.4.1 to 3.3.2

Version number indicates when the bug was initially introduced/reported.

#4 follow-up: @wonderboymusic
12 years ago

The function needs to continue to return its result by reference. The two places it contain return false should return a variable. By setting $pages to false at the top of the function, we can just return $pages instead of false.

For those who don't understand what is going on:
http://www.php.net/manual/en/language.references.return.php

When you "return by reference," you have to actually return a variable:

// returns by reference
function &get_pages( $args ) { return $pages; }

// $my_var becomes a symbol table alias for $pages
$my_var = get_pages();

The other way to make $my_var a reference:

$my_var =& get_pages();


Last edited 12 years ago by wonderboymusic (previous) (diff)

#5 in reply to: ↑ 4 @nacin
12 years ago

Replying to wonderboymusic:

The function needs to continue to return its result by reference.

Could you explain why?

#6 @wonderboymusic
12 years ago

"need" may be the wrong word - since $pages isn't bound to a global, who cares if it's passed by reference? Cuz altering it won't change any internal variables. But the bug is that a scalar is returned instead of a variable, which is easily fixed without removing return by ref.

#7 @nacin
12 years ago

  • Resolution set to fixed
  • Status changed from reviewing to closed

In [21550]:

Always return a variable reference from get_pages(). props wonderboymusic, foxinni. fixes #20756.

#8 @nacin
12 years ago

At some point we should investigate all of our return-by-reference functions and evaluate which ones are still needed. For now, [21550] fixes the issue. The rest is for another ticket.

#9 @Viper007Bond
12 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

I'm still getting this error.

Notice: Only variable references should be returned by reference in .../wp-includes/post.php on line 3524

PHP 5.3.5.

It's due to this line:

return apply_filters( 'get_pages', $pages, $r );

And can be fixed by doing this:

$pages = apply_filters( 'get_pages', $pages, $r );
return $pages;
Last edited 12 years ago by Viper007Bond (previous) (diff)

@Viper007Bond
12 years ago

When using the cache, return a variable instead of the results of a function

#10 @nacin
12 years ago

Caused earlier by [21559].

#11 @nacin
12 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In [21567]:

Return a variable reference from get_pages(). see [21559], see #21309, fixes #20756.

Note: See TracTickets for help on using tickets.