hid: another round of general code cleanups

1 files changed, 92 insertions, 91 deletions

diff --git a/xS/main.go b/xS/main.go
index 5e99a85..bcf7bc6 100644
--- a/xS/main.go
+++ b/xS/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 {