Not a bug really, but questionable code that could become a bug in the future ...
Here's the offending line 1190 of user.module (ver. 4.7.1)
$extra = _user_forms($null, $null, $null, 'register');
It's part of the user_register() callback that displays a registration form. This invocation of _user_forms gets its value in the return of $extra instead of modifying the first argument (which is passed by reference). In this case, $null isn't set or referenced anywhere else on the site so it works, but I'd think that at minimum the second two '$null' values would better be NULL, and the first one named something less confusing (e.g., ignoreme).
I don't see the use of $null anywhere else in the modules code, so I presume this wasn't a coding convention that anyone actually intended ...
As a side note - the use of this user_register() function is interesting in that it behaves differently if called from different urls. In particular, the profile module only displays its custom profile fields when users register on the normal user/register page, but the og module offers it's subscription links regardless. Kind of funky and obscure, really. In my case, I'm using the theme_user_register mechanism to make registration on a group page ask for different info from a full-on site registration. I'd be curious if others have already addressed this problem in a different way. Makes me dream of a more sophisticated registration code interface (notice I didn't call it an api!).
| Comment | File | Size | Author |
|---|---|---|---|
| #1 | usermodule_0.patch | 377 bytes | jax |
Comments
Comment #1
jax commentedI agree that it is questionable code, since the variable is not set anywhere. But because $null is local to the function the issue of it becoming used for something else is minimal. Also, if you run with "error_reporting = E_ALL", which I think one should for development, this generates a notice.
Anyway, I agree that variables should be 'set' before being used, therefore the attached patch. It makes the code more readable and you know when the variable is being used for the first time.
Comment #2
RobRoy commentedI've actually covered this in my Code Cleanup patch which needs some review: http://drupal.org/node/73884
Comment #3
jax commentedClosing this since it has been addressed in the other patch. But I do want to mention that TRUE, true, FALSE, false, NULL and null is used inconsistently throughout the code. Maybe there should be coding conventions for reserver keywords as well?
+1 for
false
true
null
(it ain't constants, it are keywords)