#170 redundant code in VERSION

u2.10.12
closed-fixed
Entrope
Server (143)
5
2016-10-21
2010-03-09
wiebe
No

The fuctions mo_version and ms_version have redundant code, which attempts to find the target server given the input.

In ms_version this never gets used as MyConnect(sptr) cannot be true.

In mo_version this is unneeded, as hunt_server_cmd takes care of finding the desired target server and error reporting. This also happens to break a nifty feature of hunt_server_cmd, namely the nick->server translation, allowing a nick to specified as the target server, and thus allowing /VERSION <nick> to be accepted, something which works for many other commands, e.g. /TIME.

Furthermore, it seems odd to check feature_int(FEAT_HIS_REMOTE) in mo_version, sptr is an oper by definition, and this restriction only applies to ordinary users. This also seems to be the case in more mo_ functions, such as mo_admin.

Slightly off-topic, but perhaps once there was only a m_version and m_admin function, for both ordinary users and opers, where it makes perfect sense to check FEAT_HIS_REMOTE. I am wondering, was this change on purpose to disallow ordinary users to issue remote admin and version requests in non-HIS configurations? If not, I think it would be great to allow this again in a non-HIS setup.

Discussion

  • Kevin L. Mitchell

    The split between the m_*, mo_*, and ms_* functions is relatively recent in ircd's history; there used to be only one function for each command. The split came about in order to simplify some of the privilege checking code, but the functions were not all revised at the time of the split, which leads to the legacy code that you note. We clean it up as we get to it, and I've considered coming up with a different way of doing privilege checking anyway, but that's still in the distant future. (Note, though, that the m_* function split preceded the features subsystem by quite a bit; the HIS_REMOTE stuff may have been due to the HIS_* coder...)

     
  • Entrope

    Entrope - 2016-10-21
    • status: open --> closed-fixed
    • assigned_to: Entrope
    • Group: --> u2.10.12
     
  • Entrope

    Entrope - 2016-10-21

    Fixed in 8d9ca11edf2117a5db1d38404b87822e88598f11. I concur that we should support /VERSION server.* when HIS_REMOTE is turned off, and the most sensible way to do that is to merge the various handlers for VERSION back into one.

     

Log in to post a comment.