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 |
| Reporter | dboyz | Assigned To | syzop | ||
| Priority | normal | Severity | trivial | Reproducibility | N/A |
| Status | resolved | Resolution | fixed | ||
| Product Version | 4.0.2 | ||||
| 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. -i- "cptr" is found to be a server [1] -ii- do_cmd is used to call CMD_FUNC(m_user) [2] -iii- However, we have the following condition checking which is _supposed_ to "return ()" because of -i- [3] -iv- Also, M_SERVER is not specified in the CommandAdd() function call for CMD_FUNC(m_user) [4] -v- Lastly, -iii- is _supposed_ to be denied as per below because of -iv- [5] References: [1] - https://github.com/unrealircd/unrealircd/blob/unreal40/src/modules/m_nick.c#L481 [2] - https://github.com/unrealircd/unrealircd/blob/unreal40/src/modules/m_nick.c#L486 [3] - https://github.com/unrealircd/unrealircd/blob/unreal40/src/modules/m_user.c#L73 [4] - https://github.com/unrealircd/unrealircd/blob/unreal40/src/modules/m_user.c#L40 [5] - 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. |
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 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 |
