WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 4 years ago

Last modified 3 years ago

#13319 closed defect (bug) (fixed)

Code improvement in _fill_single_user(): Pass by Reference PHP 4 fixes

Reported by: hakre Owned by:
Milestone: 3.0 Priority: normal
Severity: trivial Version: 3.0
Component: General Keywords: has-patch
Focuses: Cc:

Description

Passing an array by reference does not make very much sense when it is used read only. It is read only in _fill_single_user() (PHP uses copy on write, not copy on read).

I think what intended was, that objects in that array (the items in there are an array of database result objects) are passed by reference so that they play nicely with PHP 4. To achieve this, care must be taken with the iterator that it does not create copies of the objects in that array of result objects there.

maybe_unserialize() should be safe to use since database objects meta_value's property should contain scalars only.

The same is for get_user_metavalues() which is not reflecting the propper iteration over the resultset.

This all then escalated up to _fill_many_users() which is not properly handling those results as well.

Additional to those reference related fixes, some code-cleanup has been done according to the standard.

I had no problems running this on my testbed.

Attachments (2)

13319.patch (3.6 KB) - added by hakre 4 years ago.
13319.2.patch (372 bytes) - added by miqrogroove 4 years ago.
Here's my take on it.

Download all attachments as: .zip

Change History (10)

hakre4 years ago

comment:1 hakre4 years ago

References: [13576] #11914

comment:2 follow-up: nacin4 years ago

  • Cc miqrogroove added

I'm not sure I see the bug here.

comment:3 voyagerfan57614 years ago

  • Cc WordPress@… added

comment:4 in reply to: ↑ 2 hakre4 years ago

Replying to nacin:

I'm not sure I see the bug here.

No problem, there is a collection of some more resources about PHP variable handling so that we can educate ourselves on how to write PHP code for wordpress PHP requirements. Often I do not fully understand that out of the box as well I must admit. It's just not something easy to get I assume, this PHP variable handling. At least not for me even I do PHP coding since some years now. It can become quite tricky, especially with PHP 4.

miqrogroove4 years ago

Here's my take on it.

comment:5 miqrogroove4 years ago

  • Severity changed from normal to trivial

An extra param line snuck into one of the doc blocks. The ticket description did not include a reproducible bug.

comment:6 ryan4 years ago

(In [14712]) Update _fill_many_users() phpdoc. Props miqrogroove. see #13319

comment:7 nacin4 years ago

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

comment:8 hakre3 years ago

Reference: #16767

Note: See TracTickets for help on using tickets.