Yet again another php 5.3.x issue:

warning: Parameter 1 to menu_nodeapi() expected to be a reference, value given in ../includes/module.inc on line 462

CommentFileSizeAuthor
#12 i18nmenu.module.menu_nodeapi.patch476 bytessetvik

Comments

berdir’s picture

berdir
Primary language German
Location Switzerland
commented

As I said, this is not a core bug. Instead, a module calls this function in a wrong way.

Search for "module_invoke('menu', 'nodeapi'" in your code.

heine’s picture

heine commented
Status: Active » Postponed (maintainer needs more info)

Can you provide steps to reproduce? If possible, try to obtain and include a backtrace (via xdebug or debug_backtrace()).

damien tournoud’s picture

damien tournoud commented
Status: Postponed (maintainer needs more info) » Closed (won't fix)

Fix the culprit module, and open an issue in its queue.

klonos’s picture

klonos
he/him
Primary language English
Location 90% Melbourne, Australia - 10% Larissa, Greece
commented
Project: Drupal core » Internationalization
Version: 6.19 » 6.x-1.x-dev
Component: node.module » Code
Status: Closed (won't fix) » Active

Sorry for not getting to this earlier. Heres my results from the search:

grep -r "module_invoke('menu', 'nodeapi'" /var/www/test-site
/var/www/test-site/sites/all/modules/i18n/i18nmenu/i18nmenu.module:  module_invoke('menu', 'nodeapi', $node, 'prepare');

@Berdir, post #2: So, Sascha, does this seem to you like the source of my issue? If yes, do you have any idea as to how it might be fixed?

PS: turning this to the suspected module's issue queue.

klonos’s picture

klonos
he/him
Primary language English
Location 90% Melbourne, Australia - 10% Larissa, Greece
commented
Component: Code » Menus

...and a guess on the right component too.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented

Yes, this looks like it's the problem. Since i18nmenu depends on menu.module, you can expect that the function exists and simply call it directly:

menu_nodeapi($node, 'prepare');
klonos’s picture

klonos
he/him
Primary language English
Location 90% Melbourne, Australia - 10% Larissa, Greece
commented

Thanx for taking the time to reply. Last question(s), just to clarify things... do you propose I should change this:

function _i18nmenu_node_prepare($node) {
  module_invoke('menu', 'nodeapi', $node, 'prepare');
}

in i18nmenu.module (~line 350) to:

function _i18nmenu_node_prepare($node) {
  menu_nodeapi($node, 'prepare');
}

???

Also I am not sure if this is a php 5.3 issue anymore(?). Should the related tags be removed?

klonos’s picture

klonos
he/him
Primary language English
Location 90% Melbourne, Australia - 10% Larissa, Greece
commented

...just to let people know, #7 solved the issue. I guess we can check if the core menu module is available/enabled simply by doing a if module_exists in _i18nmenu_node_prepare, but I honestly wouldn't know.

And to reply to Heine's question in #2... it happens when simply trying to switch to 'edit' mode of a node of the default 'page' content type. Actually, it is a translation of a node. It happens only on trying to edit the translation and not when editing the source (original node).

If you still cannot reproduce it, let me know and I'll post a list of related modules used + the patches applied to them.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented

The difference is that PHP 5.3 is more strict:

PHP 5.2 and older: Oh, this argument is defined by reference but it is called by value. Well, let's just use it by value for this call and carry on. The result is that it actually doesn't work as expected, because $node is an object however, it will still work in PHP 5+ because objects are by reference there anyway. But it really is broken in PHP4 (Additionally, the i18nmenu_node_prepare() function doesn't define $node as by reference, so it doesn't work in PHP 4 anyway).

PHP 5.3: Oh, this argument is defined by reference but it is called by value. This wrong, you are not allowed to do that. I'm outputting a warning message and set $node to NULL so that you are forced to fix it.

The result is that this fails hard and loud in PHP 5.3 (instead of hidden and silent like in older versions) but it is broken there as well.

@klonos: I suggest that you make a patch of the change and upload it :)

klonos’s picture

klonos
he/him
Primary language English
Location 90% Melbourne, Australia - 10% Larissa, Greece
commented

Thanx for explaining this Sascha. So, tags remain ;)

I have no problem providing a patch, but does this change provide an actual solution, or is it a simple workaround? Would it effect people still in php 5.2 and prior (in a negative way)? Should we simply drop check for the core menu module or will this come and bite as in the butt latter?

berdir’s picture

berdir
Primary language German
Location Switzerland
commented

No, this doesn't have any negative effects.

The check for menu.module is not necessary because i18nmenu.info contains this line:

dependencies[]=menu

So, you can not install the module without installing menu.module as well.

setvik’s picture

setvik commented
StatusFileSize
new i18nmenu.module.menu_nodeapi.patch476 bytes

Here's a patch:

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Active » Reviewed & tested by the community

Looks good to me.

jose reyero’s picture

jose reyero commented
Status: Reviewed & tested by the community » Fixed

Fixed, thanks.

Status: Fixed » Closed (fixed)

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

erantone’s picture

erantone commented

A patch should be done for PHP 5.3 systems (see below). This hack should solve any issue, with any module expecting an argument as reference in the call. In this way, you don't have to change every single module out there.

File from drupal core: includes/module.inc 

function module_invoke() {
  $args = func_get_args();
  $module = $args[0];
  $hook = $args[1];
  unset($args[0], $args[1]);
  // START hack / http://www.php.net/manual/en/function.call-user-func-array.php#91503
  $Args = array();
  foreach($args as $k => &$arg){
    $Args[$k] = &$arg;
  }
  // END hack
  $function = $module .'_'. $hook;
  if (module_hook($module, $hook)) {
    return call_user_func_array($function, $args);
  }
}
klonos’s picture

klonos
he/him
Primary language English
Location 90% Melbourne, Australia - 10% Larissa, Greece
commented

Hey Eric, don't you think this should be filed as a separate issue against drupal core? I mean, it does solve in a generic way an issue faced by many people that has been filed in various issue queues for different modules:

http://drupal.org/project/issues?text=parameter+expected+to+be+reference...

I think that this should not take for granted that the system is installed in a php5.3 environment. This would mean that this solution will only exist as a hack applied by some people. It should instead by in core, checking the php version and acting based on it being 5.3.x and up or 5.2.x and lower. If 5.3.x and higher, then keep '&$' where it exists and add it where missing. If 5.2.x and lower do the same thing but in reverse.

All this of course for D6 & D7.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented

That is just a hack to hide the warnings, it doesn't fix anything.