Hook functions that are called from module_invoke() or module_invoke_all() should not expect their parameters to be passed by reference.

See for example the notes for user_module_invoke:

We cannot use module_invoke() for this, because the arguments need to be passed by reference.

Minimum procedure to generate an error notice:

  1. Upload Drupal-7 on a webserver running PHP-5.3
  2. Choose "Minimal" install profile.
  3. Go to /admin/modules and activate the following modules:
    • Image
    • File
    • Field UI
    • Filefield paths
    • Token
  4. Go to admin/structure/types/add and set the title to "Image", then click the (Save and add fields) button.
  5. Set the Label, name, field, and widget to "Image", "image", "Image", and "Image" respectively. Click on the (Save) button.

The following notice will be displayed:

  • Warning: Parameter 1 to filefield_paths_file_insert() expected to be a reference, value given in module_invoke_all() (line 818 of includes/module.inc).

I have the associated database dump and backtrace if anyone needs to see it.

CommentFileSizeAuthor
#18 filefield_paths-reference_variables-1005574-18.patch1.59 KBpillarsdotnet
#8 filefield_paths-fix_reference_variable_handling-1005574-8.patch1.56 KBpillarsdotnet
#1 1005574-filefield_paths.module.patch499 bytespillarsdotnet

Comments

pillarsdotnet’s picture

pillarsdotnet commented
StatusFileSize
new 1005574-filefield_paths.module.patch499 bytes

Minimum and necessary fix is to simply remove the reference from the function signature.

czigor’s picture

czigor commented

#1 works for me.

pillarsdotnet’s picture

pillarsdotnet commented
Issue tags: +Quick fix, +quickfix, +php5.3

Tagging.

mooselimb’s picture

mooselimb commented

#1 worked for me too

thanks pillarsdotnet :)

pillarsdotnet’s picture

pillarsdotnet commented
Status: Needs review » Reviewed & tested by the community
pancho’s picture

pancho
Location UTC+2 🇪🇺 EU
commented

worked for me, too. RTBC.

Tim Jones Toronto’s picture

Tim Jones Toronto commented

Yes patch solves this problem.

If anyone could help, I still have error message:

Strict warning: Only variables should be passed by reference in filefield_paths_filefield_paths_process_file() (line 475 of C:\xampp\htdocs\sites\all\modules\filefield_paths\filefield_paths.module).

** As reported as issue here: http://drupal.org/node/1090334

Thank you.

pillarsdotnet’s picture

pillarsdotnet commented
StatusFileSize
new filefield_paths-fix_reference_variable_handling-1005574-8.patch1.56 KB

Another error in reference handling. Combined patch attached.

pillarsdotnet’s picture

pillarsdotnet commented
Status: Reviewed & tested by the community » Needs review
Tim Jones Toronto’s picture

Tim Jones Toronto commented

Thanks - patch/fix #8 works fine.

pillarsdotnet’s picture

pillarsdotnet commented
Status: Needs review » Reviewed & tested by the community
johnv’s picture

johnv
Primary language Dutch
commented
Title: PHP 5.3 compatibility issue » PHP 5.3 compatibility issue: Only variables should be passed by reference

cross-post: marking #1090334: Strict warning: Only variables should be passed by reference in filefield_paths_filefield_paths_process_file() as duplicate.

pancho’s picture

pancho
Location UTC+2 🇪🇺 EU
commented

Note that in the other issue there is also a patch that might be worth merging in.

johnv’s picture

johnv
Primary language Dutch
commented

The patch from #1090334 is already in #8. Both are created by Pillarsdotnet.

pancho’s picture

pancho
Location UTC+2 🇪🇺 EU
commented

Ah okay, you're right! Just wanted to make sure that nothing gets lost.

joelstein’s picture

joelstein commented

#8 works for me.

aidanlis’s picture

aidanlis commented
Issue tags: -Quick fix, -quickfix, -php5.3

@pillarsdotnet thanks so much for your work on this, are you able to re-roll #8 for me if necessary?

pillarsdotnet’s picture

pillarsdotnet commented
StatusFileSize
new filefield_paths-reference_variables-1005574-18.patch1.59 KB

Re-roll as requested against fresh 7.x-1.x checkout.

aidanlis’s picture

aidanlis commented

The new patch contains only whitespace changes ...

pillarsdotnet’s picture

pillarsdotnet commented

Not true. Read it again.

aidanlis’s picture

aidanlis commented
Status: Reviewed & tested by the community » Fixed

Oh I see now, file_prepare_directory's got the reference requirement. Okay, thanks for the patch.

Status: Fixed » Closed (fixed)

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

dman’s picture

dman commented

Um, this is marked as fixed but I'm not seeing it in the current 'recommended' 7.x-1.0-beta3 2012-Feb-07

Looks like it's in -dev however.
http://drupalcode.org/project/filefield_paths.git/commitdiff/84fb63726b5...

I can confirm that -dev works as desired and fixes this.

Is it time for a version release yet?

burningdog’s picture

burningdog commented
Status: Closed (fixed) » Reviewed & tested by the community

dman is correct - the patch isn't in 7.x-1.0-beta3 but IS in -dev (which fixes the issue). Please create a new beta release.

burningdog’s picture

burningdog commented
Status: Reviewed & tested by the community » Closed (fixed)

Sorry, let me open a new issue for this rather.