View Issue Details
|ID||Project||Category||View Status||Date Submitted||Last Update|
|0004620||unreal||ircd||public||2016-04-04 05:10||2019-10-27 10:13|
|Fixed in Version||5.0.0-beta1|
|Summary||0004620: Use proper function instead of do_cmd() in NICK/UID|
|Description||Some of the parameters received from NICK/UID is handled by calling "CMD_FUNC(m_user)" through "do_cmd()". However I found the codes to be logically conflicting and confusing. Essentially a well commented and proper function should be the way to go.|
Note: The oddness apply to both NICK and UID although only NICK is shown below.
"cptr" is found to be a server 
do_cmd is used to call CMD_FUNC(m_user) 
However, we have the following condition checking which is _supposed_ to "return ()" because of -i- 
Also, M_SERVER is not specified in the CommandAdd() function call for CMD_FUNC(m_user) 
Lastly, -iii- is _supposed_ to be denied as per below because of -iv- 
 - https://github.com/unrealircd/unrealircd/blob/unreal40/src/modules/m_nick.c#L481
 - https://github.com/unrealircd/unrealircd/blob/unreal40/src/modules/m_nick.c#L486
 - https://github.com/unrealircd/unrealircd/blob/unreal40/src/modules/m_user.c#L73
 - https://github.com/unrealircd/unrealircd/blob/unreal40/src/modules/m_user.c#L40
 - https://github.com/unrealircd/unrealircd/blob/unreal40/src/parse.c#L376
# Perhaps there is something that I didn't quite understand about do_cmd().
# tl;dr - Again, a well commented and proper function should be used instead of what we currently have here.
|Tags||No tags attached.|
|3rd party modules|
As mentioned, the oddness listed above might not be an issue due to my misinterpretation.
To save everyone's time, I decided to clarify a bit on how I interpret the code. That way, we can immediately determine if the oddness is a valid bug or not.
> "cptr" mentioned in -i- is the pointer pointing to aClient struct. In this case, it belongs to the server connected to us. The server is also introducing users to us.
yeah that calling USER etc is rather ugly. However, cleaning it up is a major operation and risky. I don't see that happening in 4.0.x (now stable). It can happen in a 4.1.x but that is not yet open.
So I suggest to keep this issue open & in the back of our minds for our next list of code cleanups.
As for the direct issue you mention, sorry I haven't walked through your logic. I went for the short tl;dr version :D
This is now cleaned up in U5.
|2016-04-04 05:10||dboyz||New Issue|
|2016-04-04 07:37||dboyz||Note Added: 0019193|
|2016-04-09 09:31||syzop||Note Added: 0019213|
|2019-10-27 10:13||syzop||Assigned To||=> syzop|
|2019-10-27 10:13||syzop||Status||new => resolved|
|2019-10-27 10:13||syzop||Resolution||open => fixed|
|2019-10-27 10:13||syzop||Fixed in Version||=> 5.0.0-beta1|
|2019-10-27 10:13||syzop||Note Added: 0021062|