The current version of update.compare.inc/_update_process_info_list() takes &$list as second parameter, but the code never modifies the value of $list, so this can be removed.

In addition, this causes an error under E_ALL | E_STRICT, because update_get_projects passes the result of two function calls as the values for this parameter, and function returns cannot be passed by reference, so it is necessary to either use a temporary variable or remove the reference in _update_process_info_list().

The suggested patch removes the reference.

CommentFileSizeAuthor
#2 update.compare.patch719 bytesfgm
update.compare.patch674 bytesfgm
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

cburschka’s picture

Note for other testers: This patch is not made from drupal root, but from modules/update. fgm, please diff from the root directory in the future, it makes things a lot more convenient. :)

Patch applies and causes no errors.

However, I'm curious about this:

In addition, this causes an error under E_ALL | E_STRICT

I wasn't able to trigger a notice by making the unpatched module check for updates... when should this happen?

fgm’s picture

FileSize
719 bytes

Without the patch, and using PHP 5.2.3-1, just going to admin/settings causes the two warnings:

strict warning: Only variables should be passed by reference in /usr/local/src/drupal/60/modules/update/update.compare.inc on line 26.
strict warning: Only variables should be passed by reference in /usr/local/src/drupal/60/modules/update/update.compare.inc on line 27.

Here is the patch rerolled from site root

cburschka’s picture

Status: Needs review » Reviewed & tested by the community

I'm beginning to think the problem might be on my end.


error_reporting(E_STRICT);
var_dump(ini_get('error_reporting'));  // 2048

function test(&$i) {
  $i++;
  return $i;
}

function test2($i) {
  return $i;
}

var_dump(test(test2(0))); // *should* print a strict warning as return value passed by reference.
# var_dump(test(0)); // *does* print a fatal error, as literal passed by reference.

In fact, there will be no error output on my site from the above code. So I guess my test results should be ignored.

In any case, the patch has no side effects, it still applies cleanly, solves a redundancy and will bring the code toward E_STRICT compliance. So I'm going to go ahead and mark this ready.

Gábor Hojtsy’s picture

Status: Reviewed & tested by the community » Fixed

Committed.

Anonymous’s picture

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for two weeks with no activity.