mlog: make the map returned from a KVer be read-only and allowed to be nil

mlog/mlog.go

@@ -76,7 +76,7 @@ var (
 // KVer is used to provide context to a log entry in the form of a dynamic set
 // of key/value pairs which can be different for every entry.
 //
-// Each returned map should be modifiable.
+// The returned map is read-only, and may be nil.
 type KVer interface {
 	KV() map[string]interface{}
 }
@@ -94,19 +94,16 @@ type KV map[string]interface{}
 
 // KV implements the KVer method by returning a copy of itself.
 func (kv KV) KV() map[string]interface{} {
-	nkv := make(map[string]interface{}, len(kv))
+	return map[string]interface{}(kv)
-	for k, v := range kv {
-		nkv[k] = v
-	}
-	return nkv
 }
 
 // Set returns a copy of the KV being called on with the given key/val set on
 // it. The original KV is unaffected
 func (kv KV) Set(k string, v interface{}) KV {
-	nkv := kv.KV()
+	kvm := make(map[string]interface{}, len(kv)+1)
-	nkv[k] = v
+	copyM(kvm, kv.KV())
-	return nkv
+	kvm[k] = v
+	return KV(kvm)
 }
 
 // returns a key/value map which should not be written to. saves a map-cloning
@@ -120,16 +117,23 @@ func readOnlyKVM(kver KVer) map[string]interface{} {
 	return kver.KV()
 }
 
+func copyM(dst, src map[string]interface{}) {
+	for k, v := range src {
+		dst[k] = v
+	}
+}
+
 // this may take in any amount of nil values, but should never return nil
 func mergeInto(kv KVer, kvs ...KVer) map[string]interface{} {
-	if kv == nil {
+	kvm := map[string]interface{}{}
-		kv = KV(nil) // will return empty map when KV is called on it
+	if kv != nil {
+		copyM(kvm, kv.KV())
 	}
-	kvm := kv.KV()
 	for _, innerKV := range kvs {
-		for k, v := range readOnlyKVM(innerKV) {
+		if innerKV == nil {
-			kvm[k] = v
+			continue
 		}
+		copyM(kvm, innerKV.KV())
 	}
 	return kvm
 }
@@ -321,9 +325,10 @@ func (l *Logger) WithKV(kvs ...KVer) *Logger {
 	return l
 }
 
-// Log can be used to manually log a message of some custom defined Level. kvs
+// Log can be used to manually log a message of some custom defined Level.
-// will be Merge'd automatically. If the Level is a fatal (Uint() == 0) then
+//
-// calling this will never return, and the process will have os.Exit(1) called.
+// If the Level is a fatal (Uint() == 0) then calling this will never return,
+// and the process will have os.Exit(1) called.
 func (l *Logger) Log(msg Message) {
 	if l.maxLevel < msg.Level.Uint() {
 		return

mlog/mlog_test.go

@@ -21,27 +21,17 @@ func TestTruncate(t *T) {
 func TestKV(t *T) {
 	var kv KV
 	massert.Fatal(t, massert.All(
-		massert.Not(massert.Nil(kv.KV())),
+		massert.Nil(kv.KV()),
 		massert.Len(kv.KV(), 0),
 	))
 
-	// test that the KV method returns a copy
-	kv = KV{"foo": "a"}
-	kvm := kv.KV()
-	kv["bur"] = "b"
-	kvm["bar"] = "bb"
-	massert.Fatal(t, massert.All(
-		massert.Equal(KV{"foo": "a", "bur": "b"}, kv),
-		massert.Equal(map[string]interface{}{"foo": "a", "bar": "bb"}, kvm),
-	))
-
 	// test that the Set method returns a copy
 	kv = KV{"foo": "a"}
-	kvm = kv.Set("bar", "wat")
+	kv2 := kv.Set("bar", "wat")
 	kv["bur"] = "ok"
 	massert.Fatal(t, massert.All(
 		massert.Equal(KV{"foo": "a", "bur": "ok"}, kv),
-		massert.Equal(map[string]interface{}{"foo": "a", "bar": "wat"}, kvm),
+		massert.Equal(KV{"foo": "a", "bar": "wat"}, kv2),
 	))
 }