meta: fix data race caused by global buffer

Use function local backing array instead. As the size of
the function local backing array is known at compile time,
hopefully it can be allocated on the stack, to avoid heap
allocations.

This issue was identified by @zachorosz.

Fixes #56.

meta/cuesheet.go

@@ -1,12 +1,12 @@
 package meta
 
 import (
-	"bytes"
 	"encoding/binary"
 	"errors"
 	"fmt"
 	"io"
 	"io/ioutil"
+	"strings"
 )
 
 // A CueSheet describes how tracks are laid out within a FLAC stream.
@@ -30,12 +30,12 @@ type CueSheet struct {
 func (block *Block) parseCueSheet() error {
 	// Parse cue sheet.
 	// 128 bytes: MCN.
-	buf, err := readBytes(block.lr, 128)
+	szMCN, err := readString(block.lr, 128)
 	if err != nil {
 		return unexpected(err)
 	}
 	cs := &CueSheet{
-		MCN: stringFromSZ(buf),
+		MCN: stringFromSZ(szMCN),
 	}
 	block.Body = cs
 
@@ -130,11 +130,11 @@ func (block *Block) parseTrack(cs *CueSheet, i int, uniq map[uint8]struct{}) err
 	}
 
 	// 12 bytes: ISRC.
-	buf, err := readBytes(block.lr, 12)
+	szISRC, err := readString(block.lr, 12)
 	if err != nil {
 		return unexpected(err)
 	}
-	track.ISRC = stringFromSZ(buf)
+	track.ISRC = stringFromSZ(szISRC)
 
 	// 1 bit: IsAudio.
 	var x uint8
@@ -200,14 +200,14 @@ func (block *Block) parseTrack(cs *CueSheet, i int, uniq map[uint8]struct{}) err
 	return nil
 }
 
-// stringFromSZ converts the provided byte slice to a string after terminating
-// it at the first occurrence of a NULL character.
-func stringFromSZ(buf []byte) string {
-	pos := bytes.IndexByte(buf, 0)
+// stringFromSZ returns a copy of the given string terminated at the first
+// occurrence of a NULL character.
+func stringFromSZ(szStr string) string {
+	pos := strings.IndexByte(szStr, '\x00')
 	if pos == -1 {
-		return string(buf)
+		return szStr
 	}
-	return string(buf[:pos])
+	return string(szStr[:pos])
 }
 
 // CueSheetTrack contains the start offset of a track and other track specific

meta/picture.go

@@ -67,23 +67,23 @@ func (block *Block) parsePicture() error {
 	}
 
 	// (MIME type length) bytes: MIME.
-	buf, err := readBytes(block.lr, int(x))
+	mime, err := readString(block.lr, int(x))
 	if err != nil {
 		return unexpected(err)
 	}
-	pic.MIME = string(buf)
+	pic.MIME = mime
 
 	// 32 bits: (description length).
 	if err = binary.Read(block.lr, binary.BigEndian, &x); err != nil {
 		return unexpected(err)
 	}
 
 	// (description length) bytes: Desc.
-	buf, err = readBytes(block.lr, int(x))
+	desc, err := readString(block.lr, int(x))
 	if err != nil {
 		return unexpected(err)
 	}
-	pic.Desc = string(buf)
+	pic.Desc = desc
 
 	// 32 bits: Width.
 	if err = binary.Read(block.lr, binary.BigEndian, &pic.Width); err != nil {

meta/reader.go

@@ -2,25 +2,23 @@ package meta
 
 import "io"
 
-// readBuf is the local buffer used by readBytes.
-var readBuf = make([]byte, 4096)
-
-// readBytes reads and returns exactly n bytes from the provided io.Reader. The
-// local buffer is reused in between calls to reduce garbage generation. It is
-// the callers responsibility to make a copy of the returned data. The error is
-// io.EOF only if no bytes were read. If an io.EOF happens after reading some
-// but not all the bytes, ReadFull returns io.ErrUnexpectedEOF. On return, n ==
-// len(buf) if and only if err == nil.
+// readString reads and returns exactly n bytes from the provided io.Reader.
 //
-// The local buffer is initially 4096 bytes and will grow automatically if so
-// required.
-func readBytes(r io.Reader, n int) ([]byte, error) {
+// The error is io.EOF only if no bytes were read. If an io.EOF happens after
+// reading some but not all the bytes, ReadFull returns io.ErrUnexpectedEOF. On
+// return, n == len(buf) if and only if err == nil.
+func readString(r io.Reader, n int) (string, error) {
+	// readBuf is the local buffer used by readBytes.
+	var backingArray [4096]byte // hopefully allocated on stack.
+	readBuf := backingArray[:]
 	if n > len(readBuf) {
+		// The local buffer is initially 4096 bytes and will grow automatically if
+		// so required.
 		readBuf = make([]byte, n)
 	}
 	_, err := io.ReadFull(r, readBuf[:n])
 	if err != nil {
-		return nil, err
+		return "", err
 	}
-	return readBuf[:n:n], nil
+	return string(readBuf[:n]), nil
 }

meta/vorbiscomment.go

@@ -26,13 +26,13 @@ func (block *Block) parseVorbisComment() (err error) {
 	}
 
 	// (vendor length) bits: Vendor.
-	buf, err := readBytes(block.lr, int(x))
+	vendor, err := readString(block.lr, int(x))
 	if err != nil {
 		return unexpected(err)
 	}
 	comment := new(VorbisComment)
 	block.Body = comment
-	comment.Vendor = string(buf)
+	comment.Vendor = vendor
 
 	// Parse tags.
 	// 32 bits: number of tags.
@@ -50,11 +50,10 @@ func (block *Block) parseVorbisComment() (err error) {
 		}
 
 		// (vector length): vector.
-		buf, err = readBytes(block.lr, int(x))
+		vector, err := readString(block.lr, int(x))
 		if err != nil {
 			return unexpected(err)
 		}
-		vector := string(buf)
 
 		// Parse tag, which has the following format:
 		//    NAME=VALUE