From a6e6c3aaffa8f25aefed5502cdb42a18124b96e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C5=99emysl=20Janouch?= Date: Tue, 31 Jul 2018 23:37:54 +0200 Subject: [PATCH] hid: another round of general code cleanups --- hid/main.go | 183 ++++++++++++++++++++++++++-------------------------- 1 file changed, 92 insertions(+), 91 deletions(-) diff --git a/hid/main.go b/hid/main.go index 5e99a85..bcf7bc6 100644 --- a/hid/main.go +++ b/hid/main.go @@ -307,7 +307,6 @@ func callSimpleConfigWriteDefault(pathHint string, table []simpleConfigItem) { // --- Configuration ----------------------------------------------------------- var configTable = []simpleConfigItem{ - // TODO: Default to the result from os.Hostname (if successful). {"server_name", "", "Server name"}, {"server_info", "My server", "Brief server description"}, {"motd", "", "MOTD filename"}, @@ -488,6 +487,8 @@ func ircParseMessage(line string) *message { } +// --- IRC token validation ---------------------------------------------------- + // Everything as per RFC 2812 const ( ircMaxNickname = 9 @@ -1028,21 +1029,20 @@ func (c *client) getTLSCertFingerprint() string { // --- IRC command handling ---------------------------------------------------- -// XXX: ap doesn't really need to be a slice. -func (c *client) makeReply(id int, ap []interface{}) string { +func (c *client) makeReply(id int, ap ...interface{}) string { s := fmt.Sprintf(":%s %03d %s ", serverName, id, c.nicknameOrStar()) a := fmt.Sprintf(defaultReplies[id], ap...) return s + a } -// XXX: This way we cannot typecheck the arguments, so we must be careful. +// XXX: This way we cannot typecheck the arguments, so we should be careful. func (c *client) sendReply(id int, args ...interface{}) { - c.send(c.makeReply(id, args)) + c.send(c.makeReply(id, args...)) } /// Send a space-separated list of words across as many replies as needed. func (c *client) sendReplyVector(id int, items []string, args ...interface{}) { - common := c.makeReply(id, args) + common := c.makeReply(id, args...) // We always send at least one message (there might be a client that // expects us to send this message at least once). @@ -1114,8 +1114,7 @@ func (c *client) sendLUSERS() { c.sendReply(RPL_LUSERME, nUsers+nServices+nUnknown, 0 /* peer servers */) } -// TODO: Rename back to ircIsThisMe for consistency with kike. -func isThisMe(target string) bool { +func ircIsThisMe(target string) bool { // Target servers can also be matched by their users if ircFnmatch(target, serverName) { return true @@ -1275,7 +1274,7 @@ var ircCapHandlers = map[string]func(*client, *ircCapArgs){ "END": (*client).handleCAPEND, } -// XXX: Maybe these also deserve to be methods for client? They operato on +// XXX: Maybe these also deserve to be methods for client? They operate on // global state, though. func ircHandleCAP(msg *message, c *client) { @@ -1418,7 +1417,7 @@ func ircHandleUSERHOST(msg *message, c *client) { } func ircHandleLUSERS(msg *message, c *client) { - if len(msg.params) > 1 && !isThisMe(msg.params[1]) { + if len(msg.params) > 1 && !ircIsThisMe(msg.params[1]) { c.sendReply(ERR_NOSUCHSERVER, msg.params[1]) return } @@ -1426,7 +1425,7 @@ func ircHandleLUSERS(msg *message, c *client) { } func ircHandleMOTD(msg *message, c *client) { - if len(msg.params) > 0 && !isThisMe(msg.params[0]) { + if len(msg.params) > 0 && !ircIsThisMe(msg.params[0]) { c.sendReply(ERR_NOSUCHSERVER, msg.params[1]) return } @@ -1435,7 +1434,7 @@ func ircHandleMOTD(msg *message, c *client) { func ircHandlePING(msg *message, c *client) { // XXX: The RFC is pretty incomprehensible about the exact usage. - if len(msg.params) > 1 && !isThisMe(msg.params[1]) { + if len(msg.params) > 1 && !ircIsThisMe(msg.params[1]) { c.sendReply(ERR_NOSUCHSERVER, msg.params[1]) } else if len(msg.params) < 1 { c.sendReply(ERR_NOORIGIN) @@ -1465,7 +1464,7 @@ func ircHandleQUIT(msg *message, c *client) { } func ircHandleTIME(msg *message, c *client) { - if len(msg.params) > 0 && !isThisMe(msg.params[0]) { + if len(msg.params) > 0 && !ircIsThisMe(msg.params[0]) { c.sendReply(ERR_NOSUCHSERVER, msg.params[0]) return } @@ -1475,7 +1474,7 @@ func ircHandleTIME(msg *message, c *client) { } func ircHandleVERSION(msg *message, c *client) { - if len(msg.params) > 0 && !isThisMe(msg.params[0]) { + if len(msg.params) > 0 && !ircIsThisMe(msg.params[0]) { c.sendReply(ERR_NOSUCHSERVER, msg.params[0]) return } @@ -1962,7 +1961,7 @@ func ircHandleNOTICE(msg *message, c *client) { } func ircHandleLIST(msg *message, c *client) { - if len(msg.params) > 1 && !isThisMe(msg.params[1]) { + if len(msg.params) > 1 && !ircIsThisMe(msg.params[1]) { c.sendReply(ERR_NOSUCHSERVER, msg.params[1]) return } @@ -2013,8 +2012,7 @@ func ircMakeRPLNAMREPLYItem(c, target *client, modes uint) string { return result } -// TODO: Consider using *client instead of string as the map key. -func ircSendRPLNAMREPLY(c *client, ch *channel, usedNicks map[string]bool) { +func ircSendRPLNAMREPLY(c *client, ch *channel, usedNicks map[*client]bool) { kind := '=' if 0 != ch.modes&ircChanModeSecret { kind = '@' @@ -2030,17 +2028,17 @@ func ircSendRPLNAMREPLY(c *client, ch *channel, usedNicks map[string]bool) { continue } if usedNicks != nil { - usedNicks[ircToCanon(client.nickname)] = true + usedNicks[client] = true } nicks = append(nicks, ircMakeRPLNAMREPLYItem(c, client, modes)) } c.sendReplyVector(RPL_NAMREPLY, nicks, kind, ch.name, "") } -func ircSendDisassociatedNames(c *client, usedNicks map[string]bool) { +func ircSendDisassociatedNames(c *client, usedNicks map[*client]bool) { var nicks []string - for canonNickname, client := range users { - if 0 == client.mode&ircUserModeInvisible && !usedNicks[canonNickname] { + for _, client := range users { + if 0 == client.mode&ircUserModeInvisible && !usedNicks[client] { nicks = append(nicks, ircMakeRPLNAMREPLYItem(c, client, 0)) } } @@ -2050,14 +2048,13 @@ func ircSendDisassociatedNames(c *client, usedNicks map[string]bool) { } func ircHandleNAMES(msg *message, c *client) { - if len(msg.params) > 1 && !isThisMe(msg.params[1]) { + if len(msg.params) > 1 && !ircIsThisMe(msg.params[1]) { c.sendReply(ERR_NOSUCHSERVER, msg.params[1]) return } if len(msg.params) == 0 { - usedNicks := make(map[string]bool) - + usedNicks := make(map[*client]bool) for _, ch := range channels { if _, present := ch.userModes[c]; present || 0 == ch.modes&(ircChanModePrivate|ircChanModeSecret) { @@ -2211,7 +2208,7 @@ func ircHandleWHOIS(msg *message, c *client) { c.sendReply(ERR_NEEDMOREPARAMS, msg.command) return } - if len(msg.params) > 1 && !isThisMe(msg.params[0]) { + if len(msg.params) > 1 && !ircIsThisMe(msg.params[0]) { c.sendReply(ERR_NOSUCHSERVER, msg.params[0]) return } @@ -2248,7 +2245,7 @@ func ircHandleWHOWAS(msg *message, c *client) { c.sendReply(ERR_NEEDMOREPARAMS, msg.command) return } - if len(msg.params) > 2 && !isThisMe(msg.params[2]) { + if len(msg.params) > 2 && !ircIsThisMe(msg.params[2]) { c.sendReply(ERR_NOSUCHSERVER, msg.params[2]) return } @@ -2283,10 +2280,10 @@ func ircHandleTOPIC(msg *message, c *client) { return } - target := msg.params[0] - ch := channels[ircToCanon(target)] + channelName := msg.params[0] + ch := channels[ircToCanon(channelName)] if ch == nil { - c.sendReply(ERR_NOSUCHCHANNEL, target) + c.sendReply(ERR_NOSUCHCHANNEL, channelName) return } @@ -2297,13 +2294,13 @@ func ircHandleTOPIC(msg *message, c *client) { modes, present := ch.userModes[c] if !present { - c.sendReply(ERR_NOTONCHANNEL, target) + c.sendReply(ERR_NOTONCHANNEL, channelName) return } if 0 != ch.modes&ircChanModeProtectedTopic && 0 == modes&ircChanModeOperator { - c.sendReply(ERR_CHANOPRIVSNEEDED, target) + c.sendReply(ERR_CHANOPRIVSNEEDED, channelName) return } @@ -2312,28 +2309,28 @@ func ircHandleTOPIC(msg *message, c *client) { ch.topicTime = time.Now() message := fmt.Sprintf(":%s!%s@%s TOPIC %s :%s", - c.nickname, c.username, c.hostname, target, ch.topic) + c.nickname, c.username, c.hostname, channelName, ch.topic) ircChannelMulticast(ch, message, nil) } -func ircTryPart(c *client, target string, reason string) { +func ircTryPart(c *client, channelName string, reason string) { if reason == "" { reason = c.nickname } - ch := channels[ircToCanon(target)] + ch := channels[ircToCanon(channelName)] if ch == nil { - c.sendReply(ERR_NOSUCHCHANNEL, target) + c.sendReply(ERR_NOSUCHCHANNEL, channelName) return } if _, present := ch.userModes[c]; !present { - c.sendReply(ERR_NOTONCHANNEL, target) + c.sendReply(ERR_NOTONCHANNEL, channelName) return } message := fmt.Sprintf(":%s@%s@%s PART %s :%s", - c.nickname, c.username, c.hostname, target, reason) + c.nickname, c.username, c.hostname, channelName, reason) if 0 == ch.modes&ircChanModeQuiet { ircChannelMulticast(ch, message, nil) } else { @@ -2363,35 +2360,34 @@ func ircHandlePART(msg *message, c *client) { reason = msg.params[1] } - for _, target := range splitString(msg.params[0], ",", true) { - ircTryPart(c, target, reason) + for _, channelName := range splitString(msg.params[0], ",", true) { + ircTryPart(c, channelName, reason) } } -// TODO: Undo the rename from channelName to target, also in ircTryPart. -func ircTryKick(c *client, target, nick, reason string) { - ch := channels[ircToCanon(target)] +func ircTryKick(c *client, channelName, nick, reason string) { + ch := channels[ircToCanon(channelName)] if ch == nil { - c.sendReply(ERR_NOSUCHCHANNEL, target) + c.sendReply(ERR_NOSUCHCHANNEL, channelName) return } if modes, present := ch.userModes[c]; !present { - c.sendReply(ERR_NOTONCHANNEL, target) + c.sendReply(ERR_NOTONCHANNEL, channelName) return } else if 0 == modes&ircChanModeOperator { - c.sendReply(ERR_CHANOPRIVSNEEDED, target) + c.sendReply(ERR_CHANOPRIVSNEEDED, channelName) return } client := users[ircToCanon(nick)] if _, present := ch.userModes[client]; client == nil || !present { - c.sendReply(ERR_USERNOTINCHANNEL, nick, target) + c.sendReply(ERR_USERNOTINCHANNEL, nick, channelName) return } message := fmt.Sprintf(":%s@%s@%s KICK %s %s :%s", - c.nickname, c.username, c.hostname, target, nick, reason) + c.nickname, c.username, c.hostname, channelName, nick, reason) if 0 == ch.modes&ircChanModeQuiet { ircChannelMulticast(ch, message, nil) } else { @@ -2594,7 +2590,7 @@ func ircHandleISON(msg *message, c *client) { } func ircHandleADMIN(msg *message, c *client) { - if len(msg.params) > 0 && !isThisMe(msg.params[0]) { + if len(msg.params) > 0 && !ircIsThisMe(msg.params[0]) { c.sendReply(ERR_NOSUCHSERVER, msg.params[0]) return } @@ -2632,7 +2628,7 @@ func ircHandleStatsCommands(c *client) { } // We need to do it this way because of an initialization loop concerning -// ircHandlers. Workaround proposed by rsc in #1817. +// ircHandlers. Workaround proposed by rsc in go #1817. var ircHandleStatsCommandsIndirect func(c *client) func init() { @@ -2656,7 +2652,7 @@ func ircHandleSTATS(msg *message, c *client) { query = msg.params[0][0] } - if len(msg.params) > 1 && !isThisMe(msg.params[1]) { + if len(msg.params) > 1 && !ircIsThisMe(msg.params[1]) { c.sendReply(ERR_NOSUCHSERVER, msg.params[0]) return } @@ -2677,7 +2673,7 @@ func ircHandleSTATS(msg *message, c *client) { } func ircHandleLINKS(msg *message, c *client) { - if len(msg.params) > 1 && !isThisMe(msg.params[0]) { + if len(msg.params) > 1 && !ircIsThisMe(msg.params[0]) { c.sendReply(ERR_NEEDMOREPARAMS, msg.command) return } @@ -2729,7 +2725,7 @@ func ircHandleDIE(msg *message, c *client) { // ----------------------------------------------------------------------------- -// TODO: Add an index for IRC_ERR_NOSUCHSERVER validation? +// TODO: Add an index for ERR_NOSUCHSERVER validation? // TODO: Add a minimal parameter count? // TODO: Add a field for oper-only commands? Use flags? var ircHandlers = map[string]*ircCommand{ @@ -2818,7 +2814,7 @@ func (c *client) onPrepared(host string, isTLS bool) { c.hostname = host c.address = net.JoinHostPort(host, c.port) - // TODO: If we've tried to send any data before now, we need to flushSendQ. + // If we tried to send any data before now, we would need to flushSendQ. go read(c) c.reading = true } @@ -2961,7 +2957,6 @@ func prepare(client *client) { isTLS = detectTLS(sysconn) } - // FIXME: When the client sends no data, we still initialize its conn. prepared <- preparedEvent{client, host, isTLS} } @@ -3126,52 +3121,58 @@ func ircInitializeMOTD() error { return scanner.Err() } -type configError struct { - name string // configuration key - err error // description of the issue +type configProcessor struct { + err error // any error that has occurred so far } -func (e *configError) Error() string { - return fmt.Sprintf("invalid configuration value for `%s': %s", - e.name, e.err) +func (cp *configProcessor) read(name string, process func(string) string) { + if cp.err != nil { + return + } + if err := process(config[name]); err != "" { + cp.err = fmt.Errorf("invalid configuration value for `%s': %s", + name, err) + } } // This function handles values that require validation before their first use, // or some kind of a transformation (such as conversion to an integer) needs // to be done before they can be used directly. func ircParseConfig() error { - // TODO: I think we could further shorten this with lambdas, doing away - // with the custom error type completely and at the same time getting rid of - // the key stuttering. - if u, err := strconv.ParseUint( - config["ping_interval"], 10, 32); err != nil { - return &configError{"ping_interval", err} - } else if u < 1 { - return &configError{"ping_interval", - errors.New("the value is out of range")} - } else { - pingInterval = uint(u) - } - - if i, err := strconv.ParseInt( - config["max_connections"], 10, 32); err != nil { - return &configError{"max_connections", err} - } else if i < 0 { - return &configError{"max_connections", - errors.New("the value is out of range")} - } else { - maxConnections = int(i) - } - - operators = make(map[string]bool) - for _, fp := range splitString(config["operators"], ",", true) { - if !ircIsValidFingerprint(fp) { - return &configError{"operators", - errors.New("invalid fingerprint valeu")} + cp := &configProcessor{} + cp.read("ping_interval", func(value string) string { + if u, err := strconv.ParseUint( + config["ping_interval"], 10, 32); err != nil { + return err.Error() + } else if u < 1 { + return "the value is out of range" + } else { + pingInterval = uint(u) } - operators[strings.ToLower(fp)] = true - } - return nil + return "" + }) + cp.read("max_connections", func(value string) string { + if i, err := strconv.ParseInt( + value, 10, 32); err != nil { + return err.Error() + } else if i < 0 { + return "the value is out of range" + } else { + maxConnections = int(i) + } + return "" + }) + cp.read("operators", func(value string) string { + operators = make(map[string]bool) + for _, fp := range splitString(value, ",", true) { + if !ircIsValidFingerprint(fp) { + return "invalid fingerprint value" + } + operators[strings.ToLower(fp)] = true + } + return "" + }) + return cp.err } func ircInitializeServerName() error {