The doc for hook_nodeapi() is pretty clear that the 3rd and 4th params must be options. However, some core implemtations make it required.


This cause's errors when invoking hook_nodeapi with just the 1st 2 params (which are all that should be required).

#5 1260932-nodeapi.patch3.49 KBDjebbZ
PASSED: [[SimpleTest]]: [MySQL] 190 pass(es).
[ View ]
#2 1260932-nodeapi.patch5.9 KBDjebbZ
PASSED: [[SimpleTest]]: [MySQL] 190 pass(es).
[ View ]


pwolanin’s picture

Title:Severla core implementations of hook_nodeapi() have required 3rd parameters» Several core implementations of hook_nodeapi() have required 3rd parameters
Issue tags:+Novice

tagging as Novice, since this is a really easy patch.

All core implenetations need to be checked, but upload_nodeapi(), trigger_nodeapi() and path_nodeapi() clearly have the problem.

DjebbZ’s picture

new5.9 KB
PASSED: [[SimpleTest]]: [MySQL] 190 pass(es).
[ View ]

Basically I fixed all implementations of hook_nodeapi in core (more than 3 !). (My first core patch)

DjebbZ’s picture

Status:Active» Needs review


pwolanin’s picture

Status:Needs review» Needs work

I think this needs work - some of the changes add a 4th param (not needed) or change the name of the 3rd param:

-function comment_nodeapi(&$node, $op, $arg = 0) {
+function comment_nodeapi(&$node, $op, $a3 = NULL, $a4 = NULL) {

I think we don't need the change above at all. The main thing is to make sure that any 3rd or 4th params defined are optional (have a default value).

For example, I would change path_nodeapi() to:

function path_nodeapi(&$node, $op, $arg = NULL) {
DjebbZ’s picture

new3.49 KB
PASSED: [[SimpleTest]]: [MySQL] 190 pass(es).
[ View ]

What about statistics module ? third parameter is 0, not NULL. Same for taxonomy.

 function statistics_nodeapi(&$node, $op, $arg = 0) {
pwolanin’s picture

Status:Needs work» Needs review

I think it's fine for them to be 0 instead of NULL at this point - as long as there is a default value, it's optional.

DjebbZ’s picture

Sorry again.

DjebbZ’s picture

Ok I understant. Cool.

pwolanin’s picture

Status:Needs review» Reviewed & tested by the community

ok, so the modified modules are:


Applied this patch, enabled all those module and did some basic functional tests. Didn't see (or expect) any errors.

Patch also looks good - minimal change to comply with the API and make 3rd and 4th params optional.

DjebbZ’s picture

So basically, i wrote my first patch for core :) /me is happy :)
I'll continue with core issues tagged "Novice", they fit me well.

gateway69’s picture

Im also seeing this issue, im curious if I should apply the patch here..

Warning: Missing argument 4 for views_attach_nodeapi(), called in /var/www/html/content/sites/all/modules/apachesolr/ on line 63 and defined in views_attach_nodeapi() (line 86 of /var/www/html/content/sites/all/modules/views_attach/views_attach.module).
pwolanin’s picture

@gateway69 - no, it looks to be a corresponding bug in the views_attach module. Please file the bug there.

Gábor Hojtsy’s picture

Status:Reviewed & tested by the community» Fixed

Ok, reviewed, looks simple and straightforward. Committed. Thanks.

DjebbZ’s picture

/me very happy :) First patch to core :)

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