WordPress.org

Make WordPress Core

Opened 8 years ago

Closed 8 years ago

#18237 closed defect (bug) (fixed)

get_role triggers a PHP notice

Reported by: elyobo Owned by: ryan
Milestone: 3.3 Priority: normal
Severity: minor Version: 3.2.1
Component: Users Keywords: has-patch
Focuses: Cc:

Description

get_role (in wp-includes/capabilities.php) incorrectly returns a null value as a reference, triggering a PHP notice "Only variable references should be returned by reference".

While this is not a terribly important problem, it is easily resolved with the patch attached (just assign the value to a variable first, then return that).

This is a more important issue for us because we develop using a custom error handler which throws ErrorExceptions on all notices. This helps to catch a range of PHP errors (e.g. typos, uninitialised variables and whatnot), but is sometimes a PITA because of things like this.

Attachments (3)

wp-fix-return-by-reference.patch (307 bytes) - added by elyobo 8 years ago.
get_roles patch
wp-fix-return-by-reference.svn.patch (536 bytes) - added by elyobo 8 years ago.
Patch as svn diff from trunk
18237.diff (501 bytes) - added by scribu 8 years ago.

Download all attachments as: .zip

Change History (9)

@elyobo
8 years ago

get_roles patch

#1 @nacin
8 years ago

  • Milestone changed from Awaiting Review to 3.3

Can you submit an svn diff, rather than a diff -u?

This looks good.

@elyobo
8 years ago

Patch as svn diff from trunk

#2 @elyobo
8 years ago

Diff is against the current SVN trunk.

#3 @scribu
8 years ago

I think making get_role() not return a reference would be a better fix, now that we're in PHP5 land.

@scribu
8 years ago

#4 @ocean90
8 years ago

Maybe we should close this ticket as a dup of #16767 and review the reference uses in core.

#5 @elyobo
8 years ago

Agreed with scribu; I was trying to keep the patch small rather than messing with the widespread use of references, but scratching the reference is a cleaner solution.

How likely is it that #16767 will get rolled in?

#6 @ryan
8 years ago

  • Owner set to ryan
  • Resolution set to fixed
  • Status changed from new to closed

In [18476]:

Remove return by ref from get_role(). Props scribu, elyobo. fixes #18237

Note: See TracTickets for help on using tickets.