[drupal-devel] [bug] module_invoke brokes default values
Project: Drupal Version: cvs Component: base system Category: bug reports Priority: critical Assigned to: chx Reported by: chx Updated by: chx Status: patch Attachment: http://drupal.org/files/issues/module_inc_no_default_nulls.patch (685 bytes) 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. chx -- View: http://drupal.org/node/17770 Edit: http://drupal.org/project/comments/add/17770
Project: Drupal Version: cvs Component: base system Category: bug reports Priority: critical Assigned to: chx Reported by: chx Updated by: killes@www.drop.org Status: patch I have no idea how else we should fix this. +1. killes@www.drop.org Previous comments: ------------------------------------------------------------------------ February 22, 2005 - 22:26 : chx Attachment: http://drupal.org/files/issues/module_inc_no_default_nulls.patch (685 bytes) 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. -- View: http://drupal.org/node/17770 Edit: http://drupal.org/project/comments/add/17770
Project: Drupal Version: cvs Component: base system Category: bug reports Priority: critical Assigned to: chx Reported by: chx Updated by: chx Status: patch Attachment: http://drupal.org/files/issues/module_inc_no_default_nulls_0.patch (625 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 Previous comments: ------------------------------------------------------------------------ February 22, 2005 - 22:26 : chx Attachment: http://drupal.org/files/issues/module_inc_no_default_nulls.patch (685 bytes) 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. ------------------------------------------------------------------------ February 22, 2005 - 22:35 : killes@www.drop.org I have no idea how else we should fix this. +1. -- View: http://drupal.org/node/17770 Edit: http://drupal.org/project/comments/add/17770
Project: Drupal Version: cvs Component: base system Category: bug reports Priority: critical Assigned to: chx Reported by: chx Updated by: chx Status: patch Attachment: http://drupal.org/files/issues/themefix_0.patch (712 bytes) The message was wrong (but funny). chx Previous comments: ------------------------------------------------------------------------ February 22, 2005 - 22:26 : chx Attachment: http://drupal.org/files/issues/module_inc_no_default_nulls.patch (685 bytes) 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. ------------------------------------------------------------------------ February 22, 2005 - 22:35 : killes@www.drop.org I have no idea how else we should fix this. +1. ------------------------------------------------------------------------ February 22, 2005 - 23:49 : chx Attachment: http://drupal.org/files/issues/module_inc_no_default_nulls_0.patch (625 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. -- View: http://drupal.org/node/17770 Edit: http://drupal.org/project/comments/add/17770
Project: Drupal Version: cvs Component: base system Category: bug reports Priority: critical Assigned to: chx Reported by: chx Updated by: chx Status: patch Sorry, posted to the wrong issue. :( chx Previous comments: ------------------------------------------------------------------------ February 22, 2005 - 22:26 : chx Attachment: http://drupal.org/files/issues/module_inc_no_default_nulls.patch (685 bytes) 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. ------------------------------------------------------------------------ February 22, 2005 - 22:35 : killes@www.drop.org I have no idea how else we should fix this. +1. ------------------------------------------------------------------------ February 22, 2005 - 23:49 : chx Attachment: http://drupal.org/files/issues/module_inc_no_default_nulls_0.patch (625 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. ------------------------------------------------------------------------ February 23, 2005 - 00:29 : chx Attachment: http://drupal.org/files/issues/themefix_0.patch (712 bytes) The message was wrong (but funny). -- View: http://drupal.org/node/17770 Edit: http://drupal.org/project/comments/add/17770
Project: Drupal Version: cvs Component: base system Category: bug reports Priority: critical Assigned to: chx Reported by: chx Updated by: Dries Status: patch The function module_invoke_all needs to be updated as well. Won't this break arguments passed by reference? Did you test such scenario? Dries Previous comments: ------------------------------------------------------------------------ February 22, 2005 - 23:26 : chx Attachment: http://drupal.org/files/issues/module_inc_no_default_nulls.patch (685 bytes) 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. ------------------------------------------------------------------------ February 22, 2005 - 23:35 : killes@www.drop.org I have no idea how else we should fix this. +1. ------------------------------------------------------------------------ February 23, 2005 - 00:49 : chx Attachment: http://drupal.org/files/issues/module_inc_no_default_nulls_0.patch (625 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. ------------------------------------------------------------------------ February 23, 2005 - 01:29 : chx Attachment: http://drupal.org/files/issues/themefix_0.patch (712 bytes) The message was wrong (but funny). ------------------------------------------------------------------------ February 23, 2005 - 01:30 : chx Sorry, posted to the wrong issue. :( -- View: http://drupal.org/node/17770 Edit: http://drupal.org/project/comments/add/17770
Project: Drupal Version: cvs Component: base system Category: bug reports Priority: critical Assigned to: chx Reported by: chx Updated by: chx Status: patch 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. chx Previous comments: ------------------------------------------------------------------------ February 22, 2005 - 22:26 : chx Attachment: http://drupal.org/files/issues/module_inc_no_default_nulls.patch (685 bytes) 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. ------------------------------------------------------------------------ February 22, 2005 - 22:35 : killes@www.drop.org I have no idea how else we should fix this. +1. ------------------------------------------------------------------------ February 22, 2005 - 23:49 : chx Attachment: http://drupal.org/files/issues/module_inc_no_default_nulls_0.patch (625 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. ------------------------------------------------------------------------ February 23, 2005 - 00:29 : chx Attachment: http://drupal.org/files/issues/themefix_0.patch (712 bytes) The message was wrong (but funny). ------------------------------------------------------------------------ February 23, 2005 - 00:30 : chx Sorry, posted to the wrong issue. :( ------------------------------------------------------------------------ February 23, 2005 - 07:16 : Dries The function module_invoke_all needs to be updated as well. Won't this break arguments passed by reference? Did you test such scenario? -- View: http://drupal.org/node/17770 Edit: http://drupal.org/project/comments/add/17770
Project: Drupal Version: cvs Component: base system Category: bug reports Priority: critical Assigned to: chx Reported by: chx Updated by: Dries Status: patch Bart's approach is preferred. Dries Previous comments: ------------------------------------------------------------------------ February 22, 2005 - 23:26 : chx Attachment: http://drupal.org/files/issues/module_inc_no_default_nulls.patch (685 bytes) 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. ------------------------------------------------------------------------ February 22, 2005 - 23:35 : killes@www.drop.org I have no idea how else we should fix this. +1. ------------------------------------------------------------------------ February 23, 2005 - 00:49 : chx Attachment: http://drupal.org/files/issues/module_inc_no_default_nulls_0.patch (625 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. ------------------------------------------------------------------------ February 23, 2005 - 01:29 : chx Attachment: http://drupal.org/files/issues/themefix_0.patch (712 bytes) The message was wrong (but funny). ------------------------------------------------------------------------ February 23, 2005 - 01:30 : chx Sorry, posted to the wrong issue. :( ------------------------------------------------------------------------ February 23, 2005 - 08:16 : Dries The function module_invoke_all needs to be updated as well. Won't this break arguments passed by reference? Did you test such scenario? ------------------------------------------------------------------------ February 23, 2005 - 08:32 : chx 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. -- View: http://drupal.org/node/17770 Edit: http://drupal.org/project/comments/add/17770
Issue status update for http://drupal.org/node/17770 Project: Drupal Version: cvs Component: base system Category: bug reports Priority: critical Assigned to: chx Reported by: chx Updated by: chx Status: patch Attachment: http://drupal.org/files/issues/module_invoke.patch (1.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 Previous comments: ------------------------------------------------------------------------ February 22, 2005 - 22:26 : chx Attachment: http://drupal.org/files/issues/module_inc_no_default_nulls.patch (685 bytes) 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. ------------------------------------------------------------------------ February 22, 2005 - 22:35 : killes@www.drop.org I have no idea how else we should fix this. +1. ------------------------------------------------------------------------ February 22, 2005 - 23:49 : chx Attachment: http://drupal.org/files/issues/module_inc_no_default_nulls_0.patch (625 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. ------------------------------------------------------------------------ February 23, 2005 - 00:29 : chx Attachment: http://drupal.org/files/issues/themefix_0.patch (712 bytes) The message was wrong (but funny). ------------------------------------------------------------------------ February 23, 2005 - 00:30 : chx Sorry, posted to the wrong issue. :( ------------------------------------------------------------------------ February 23, 2005 - 07:16 : Dries The function module_invoke_all needs to be updated as well. Won't this break arguments passed by reference? Did you test such scenario? ------------------------------------------------------------------------ February 23, 2005 - 07:32 : chx 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. ------------------------------------------------------------------------ February 23, 2005 - 09:41 : Dries Bart's approach is preferred.
Issue status update for http://drupal.org/node/17770 Project: Drupal Version: cvs Component: base system Category: bug reports Priority: critical Assigned to: chx Reported by: chx Updated by: chx Status: patch Attachment: http://drupal.org/files/issues/module_invoke_0.patch (1.92 KB) I think this one is better. chx Previous comments: ------------------------------------------------------------------------ February 22, 2005 - 22:26 : chx Attachment: http://drupal.org/files/issues/module_inc_no_default_nulls.patch (685 bytes) 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. ------------------------------------------------------------------------ February 22, 2005 - 22:35 : killes@www.drop.org I have no idea how else we should fix this. +1. ------------------------------------------------------------------------ February 22, 2005 - 23:49 : chx Attachment: http://drupal.org/files/issues/module_inc_no_default_nulls_0.patch (625 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. ------------------------------------------------------------------------ February 23, 2005 - 00:29 : chx Attachment: http://drupal.org/files/issues/themefix_0.patch (712 bytes) The message was wrong (but funny). ------------------------------------------------------------------------ February 23, 2005 - 00:30 : chx Sorry, posted to the wrong issue. :( ------------------------------------------------------------------------ February 23, 2005 - 07:16 : Dries The function module_invoke_all needs to be updated as well. Won't this break arguments passed by reference? Did you test such scenario? ------------------------------------------------------------------------ February 23, 2005 - 07:32 : chx 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. ------------------------------------------------------------------------ February 23, 2005 - 09:41 : Dries Bart's approach is preferred. ------------------------------------------------------------------------ February 28, 2005 - 20:33 : chx Attachment: http://drupal.org/files/issues/module_invoke.patch (1.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.
participants (3)
-
chx -
Dries
-
killes