From 8f4c14c7833958bce43bff8936ae67da817f8c93 Mon Sep 17 00:00:00 2001
From: Shivaram Lingamneni <slingamn@cs.stanford.edu>
Date: Thu, 20 Feb 2020 01:45:17 -0500
Subject: [PATCH] miscellaneous review fixes

---
 irc/client.go          | 8 ++++++--
 irc/handlers.go        | 2 +-
 irc/history/history.go | 9 ++++++---
 irc/mysql/history.go   | 3 +--
 4 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/irc/client.go b/irc/client.go
index 6ad8ae10..82bd20b1 100644
--- a/irc/client.go
+++ b/irc/client.go
@@ -871,8 +871,12 @@ func (client *Client) replayPrivmsgHistory(rb *ResponseBuffer, items []history.I
 		if allowTags {
 			tags = item.Tags
 		}
+		// XXX: Params[0] is the message target. if the source of this message is an in-memory
+		// buffer, then it's "" for an incoming message and the recipient's nick for an outgoing
+		// message. if the source of the message is mysql, then mysql only sees one copy of the
+		// message, and it's the version with the recipient's nick filled in. so this is an
+		// incoming message if Params[0] (the recipient's nick) equals the client's nick:
 		if item.Params[0] == "" || item.Params[0] == nick {
-			// this message was sent *to* the client from another nick
 			rb.AddSplitMessageFromClient(item.Nick, item.AccountName, tags, command, nick, item.Message)
 		} else {
 			// this message was sent *from* the client to another nick; the target is item.Params[0]
@@ -1237,9 +1241,9 @@ func (client *Client) destroy(session *Session) {
 		Message:     splitQuitMessage,
 	}
 	var channels []*Channel
+	// use a defer here to avoid writing to mysql while holding the destroy semaphore:
 	defer func() {
 		for _, channel := range channels {
-			// TODO it's dangerous to write to mysql while holding the destroy semaphore
 			channel.AddHistoryItem(quitItem)
 		}
 	}()
diff --git a/irc/handlers.go b/irc/handlers.go
index 93d567c3..314e9dd9 100644
--- a/irc/handlers.go
+++ b/irc/handlers.go
@@ -1914,7 +1914,7 @@ func dispatchMessageToTarget(client *Client, tags map[string]string, histType hi
 		var deliverySessions []*Session
 		// restrict messages appropriately when +R is set
 		// intentionally make the sending user think the message went through fine
-		allowedPlusR := !user.HasMode(modes.RegisteredOnly) || details.account != ""
+		allowedPlusR := details.account != "" || !user.HasMode(modes.RegisteredOnly)
 		if allowedPlusR {
 			deliverySessions = append(deliverySessions, user.Sessions()...)
 		}
diff --git a/irc/history/history.go b/irc/history/history.go
index b39b753a..b7317943 100644
--- a/irc/history/history.go
+++ b/irc/history/history.go
@@ -42,9 +42,12 @@ type Item struct {
 	// this is the uncasefolded account name, if there's no account it should be set to "*"
 	AccountName string
 	// for non-privmsg items, we may stuff some other data in here
-	Message         utils.SplitMessage
-	Tags            map[string]string
-	Params          [1]string
+	Message utils.SplitMessage
+	Tags    map[string]string
+	Params  [1]string
+	// for a DM, this is the casefolded nickname of the other party (whether this is
+	// an incoming or outgoing message). this lets us emulate the "query buffer" functionality
+	// required by CHATHISTORY:
 	CfCorrespondent string
 }
 
diff --git a/irc/mysql/history.go b/irc/mysql/history.go
index 5482b5e9..e9ad5b0e 100644
--- a/irc/mysql/history.go
+++ b/irc/mysql/history.go
@@ -5,7 +5,6 @@ import (
 	"database/sql"
 	"fmt"
 	"runtime/debug"
-	"strconv"
 	"sync"
 	"time"
 
@@ -197,7 +196,7 @@ func (mysql *MySQL) doCleanup(age time.Duration) (count int, err error) {
 		if i != 0 {
 			inBuf.WriteRune(',')
 		}
-		inBuf.WriteString(strconv.FormatInt(int64(id), 10))
+		fmt.Fprintf(&inBuf, "%d", id)
 	}
 	inBuf.WriteRune(')')