Try this: module_invoke('taxonomy', 'get_tree', 1); and compare the results with taxonomy_get_tree(1);

What happens is that taxonomy_get_tree is called by the arguments 1, NULL, NULL, NULL which breaks it.

Hence the patch.

Every Drupal version I have met (4.4, 4.5, CVS) contains this error. I think this led module authors to not use module_invoke (at least I did so until now) which will be quite a problem if we decide on breaking up the modules into smaller pieces.

CommentFileSizeAuthor
#9 module_invoke_0.patch1.92 KBchx
#8 module_invoke.patch1.84 KBchx
#3 themefix_0.patch712 byteschx
#2 module_inc_no_default_nulls_0.patch625 byteschx
module_inc_no_default_nulls.patch685 byteschx

Comments

killes@www.drop.org’s picture

killes@www.drop.org commented

I have no idea how else we should fix this. +1.

chx’s picture

chx commented
StatusFileSize
new module_inc_no_default_nulls_0.patch625 bytes

Of course, lambda is another solution. When I objected on IRC that call_user_func_array is slower, Bart Jansens pointed out that slashdotted pages are cached thus module_invoke is not called. So if this solution is accepted, the credit goes to him.

But one of the solutions shall be accepted.

chx’s picture

chx commented
StatusFileSize
new themefix_0.patch712 bytes

The message was wrong (but funny).

chx’s picture

chx commented

Sorry, posted to the wrong issue. :(

dries’s picture

dries commented
  • The function module_invoke_all needs to be updated as well.
  • Won't this break arguments passed by reference? Did you test such scenario?
chx’s picture

chx commented

module_invoke_all, sure, just tell me which approach is accepted (lambda or switch(func_num_args())

References? module_invoke does not accept references, that's why we have node_invoke and nove_invoke_nodeapi (these may be wrong, too) and user_invoke (this one has only one NULL, in a very specific place, so most probably it won't break anything).

So, let's agree on one way or another and I'll patch module_invoke_all, node_invoke, node_invoke_nodeapi too.

dries’s picture

dries commented

Bart's approach is preferred.

chx’s picture

chx commented
StatusFileSize
new module_invoke.patch1.84 KB

Here is a fix for module_invoke_all and module_invoke.

Turns out that the node_invoke and nove_invoke_nodeapi and user_invoke does not need replacement.

There was a related bug in user.module, where necessary NULL parameters were not in the module_invoke call, that's fixed, too.

Also, module_invoke_all uses module_implements which is good if one hook (say hook_nodeapi) is called more than once and does no harm if it is called only once.

chx’s picture

chx commented
StatusFileSize
new module_invoke_0.patch1.92 KB

I think this one is better.

dries’s picture

dries commented

Committed to HEAD.