_portable_path_field_attach_presave() is making calls to array_shift() and passing in directly the results of array_keys(). E.g:
$column = array_shift(array_keys($field['columns']));
This is incorrect use of the function and will cause warnings when strict error mode is active because only variables should be passed to array_shift().

If a patch is wanted I can create one for dev head but the fix is pretty simple. Around line 36 of _portable_path_field_attach_presave() change:

      $column = array_shift(array_keys($field['columns']));
      // Get the language key from the field array.
      $language = array_shift(array_keys($entity->$field_name));

to

      $column_keys = array_keys($field['columns']);
      $column = array_shift($column_keys);
      // Get the language key from the field array.
      $entity_keys = array_keys($entity->$field_name);
      $language = array_shift($entity_keys);
CommentFileSizeAuthor
#7 portable_path-fix_php_strict_reference_errors-2047415-7-7x.patch3.16 KBguypaddock

Comments

solotandem’s picture

solotandem commented
Status: Active » Closed (fixed)

This code was changed on 2013-06-26 to eliminate the PHP warning. The array_shift() call was replaced by current(). If this does not eliminate the warning, then reopen this issue.

solotandem’s picture

solotandem commented
Status: Closed (fixed) » Active

Oops. Wrong file. I changed the same code in _portable_path_field_attach_load() but missed this in _portable_path_field_attach_presave().

solotandem’s picture

solotandem commented
Title: array_shift() is pass by reference » Eliminate PHP strict warnings in _portable_path_field_attach_presave()
Assigned: Unassigned » solotandem
Status: Active » Fixed

Fixed in this commit.

magicmyth’s picture

magicmyth commented
Status: Fixed » Active

I'm afraid that will not fix the issue because current() is also pass by reference. I tested the above patch to make sure and got the same error message "Strict warning: Only variables should be passed by reference in[...]". The result of array_keys() must be assigned to a variable first, which is then passed to current() or array_shift().

solotandem’s picture

solotandem commented

I see that the array parameter to current() is a reference; yet i do not get a strict warning with current() while I do with array_shift(). The documentation for the current() function says it does not alter the array; so the importance of an actual variable versus a function result would not seem to be the same.

What version of php are you running?

magicmyth’s picture

magicmyth commented

PHP 5.4.14. You may not have error messages set to the highest strictness in your ini though I would be surprised that was the reason as you mentioned you do get the error with array_shift().

guypaddock’s picture

guypaddock commented
Issue summary: View changes
StatusFileSize
new portable_path-fix_php_strict_reference_errors-2047415-7-7x.patch3.16 KB

Attached is a patch that appears to correct the PHP notices with the latest release version. I had mixed results with the dev version.

I also needed to add a check on the view hook to cope with entity stubs, as happens when you are accessing the Content overview page (admin/content) after a cache clear. It looks like, on that page, the "body" field is NULL, causing an "array index not found" error to appear. Not sure why I didn't see this the last time I used this module on a another site...

solotandem’s picture

solotandem commented
Status: Active » Fixed
Related issues: +#2325441: Eliminate PHP notice in _portable_path_field_attach_save()

Regarding patch in #7, a patch against the dev release is included on the related issue. This fixes the empty body field notice on save (which had been on my radar for a few months, but not committed).

Regarding use of current() with a non-variable argument, please grep Drupal core for the same type of expression. And note that core has tests that would fail if this expression resulted in a PHP notice, etc.

Status: Fixed » Closed (fixed)

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