I am seeing regular errors in my logs along the lines of

Parameter 4 to mm_cck_media_mover() expected to be a reference, value given in /var/www/drupal-6.25/includes/module.inc on line 476

I looked in the media_mover_api.module and found one instance where module_invoke() was *NOT* called (instead using call_user_func_array), and a comment immediately preceding that line saying that module_invoke doesn't seem to pass by reference correctly :

588: // @NOTE not using module_invoke here b/c it doesn't seem to pass the $file by reference correctly ($file)
589: $filepath = call_user_func_array($config->{$verb}->module .'_media_mover', array($verb, $config->{$verb}->action, $config->{$verb}->configuration, &$file, $config));

This seems to be an acknowledgement of the call by reference problem which I am seeing, which is why I am surprised to see several subsequent calls, with the file parameter, sent through module invoke

1155: module_invoke($file["{$verb}_module"], 'media_mover', 'update', $file["{$verb}_action"], NULL, $file);

2856: if ($new_items = module_invoke($configuration->{$verb}->module, 'media_mover', 'fetch', $item_config->{$verb}->action, $configuration, $item)) {

3014: module_invoke($configuration->{$verb}->module, 'media_mover', 'delete', $configuration->{$verb}->action, $configuration->{$verb}->configuration, $file, $configuration);

3092: module_invoke($configuration->{$verb}->module, 'media_mover', 'delete', $configuration->{$verb}->action, $configuration->{$verb}->configuration, $file, $configuration);

It seems that if module_invoke() is going to generate errors like I'm seeing, then it shouldn't be used anywhere that the 4th $file parameter is being passed. I can't decide whether these subsequent calls to module_invoke are in fact the source of my problem, and a legitimate problem in the code, or whether I am just rationalizing that the problem isn't mine :=)

Does someone know why module_invoke with the $file parameter was replaced in one situation, but not in the others? Doesn't that alone seem inconsistent, and worthy of some investigation?

Comments

carteriii’s picture

carteriii commented

I should add that my errors are specifically being generated as a result of the call to module_invoke() within the function media_mover_api_run_config_harvest(). Ironically enough, the $file parameter in this call is actually NULL (see below, the 6th parameter to moduke_invoke() becomes the 4th parameter to the actual hook), but it still generating the erorr that it is expected to be a reference. For reference, here is the call within media_mover_api_run_config_harvest:

    $files = module_invoke($config->{$verb}->module, 'media_mover', $verb, $config->{$verb}->action, $config->{$verb}->configuration, NULL, $config, $nid);
carteriii’s picture

carteriii commented

As I try to debug this for myself, I think I see another problem, independent of the use of module_invoke. How can it ever be proper to pass NULL as $file parameter, which appears to always be declared as &$file = array()?

The PHP manual on passing by reference lists specific things that can be passed by reference, and NULL isn't one of them. One of the first comments on the page from "jocelyn dot heuze" also says/confirms that, "you can't pass NULL to a function by reference".

So it seems that the call to module_invoke which I listed above, which passes "NULL" as the $file parameter, should instead be passing an empty array.

Going back to the original topic and the use of module_invoke(), I have now tested and confirmed that module_invoke() does behave differently than calling call_user_func_array() directly. The call by reference appears to get lost with module_invoke's use of func_get_args(). I can get media_mover_api_run_config_harvest() to run if I remove module_invoke() and replace it with a direct call to call_user_func_array(), but otherwise it fails with the error about passing a value by reference.

I'm starting to wonder how this can work for everyone. I have PHP version 5.3.2 on one computer and 5.3.6-13 on another, both running ubuntu. Anyone have any thoughts?

carteriii’s picture

carteriii commented

I just did a diff on the 6.1 dev branch and I see that my concern about module_invoke has been addressed. All calls to module_invoke() have been replaced with a custom function _media_mover_api_module_invoke().

I believe the only potential problem with this is the use of NULL for the default parameters. As I mentioned in comment #1 above, in the call from media_mover_api_run_config_harvest, I believe NULL should not be used as the 4th parameter because that is supposed to be a call by reference with a default value of an empty array. I believe that should probably be handled in the new function _media_mover_api_module_invoke().