logdog: Add frame read/write library.

common/logdog/frame/reader.go

+// Copyright 2015 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+package frame
+
+import (
+        "encoding/binary"
+        "fmt"
+        "io"
+)
+
+// ErrFrameTooLarge is an error that is returned if a frame that is larger than
+// the maximum allowed size (not including the frame header) is read.
+var ErrFrameTooLarge = fmt.Errorf("frame: frame size exceeds maximum")
+
+// Reader reads individual frames from a frame-formatted input Reader.
+type Reader interface {
+        // ReadFrame returns the contents of the next frame. If there are no more
+        // frames available, Next will return io.EOF.
+        ReadFrame() ([]byte, error)
+}
+
+// reader is an implementation of a Reader that uses an underlying
+// io.Reader and io.ByteReader to read frames.
+//
+// The io.Reader and io.ByteReader must read from the same source.
+type reader struct {
+        io.Reader
+        io.ByteReader
+
+        maxSize int
+}

[iannucci, 2015/08/03 22:21:24]

interface assertion?

[dnj, 2015/08/03 22:42:58]

Going with vadimsh@ style: since NewReader returns a Reader interface from
&reader, the assertion isn't necessary.

+
+// NewReader creates a new Reader which reads frame data from the
+// supplied Reader instance.
+//
+// If the Reader instance is also an io.ByteReader, its ReadByte method will
+// be used directly.
+func NewReader(r io.Reader, maxSize int) Reader {
+        br, ok := r.(io.ByteReader)
+        if !ok {
+                br = &simpleByteReader{Reader: r}

[iannucci, 2015/08/03 22:21:24]

not sure if this is needed? why not just have this take interface{Reader;
ByteReader}?

[dnj, 2015/08/03 22:42:58]

It puts the burden on the caller to implement ByteReader for Reader streams.
This way, the public interface stays simple.

+        }
+        return &reader{
+                Reader:     r,
+                ByteReader: br,
+                maxSize:    maxSize,
+        }
+}
+
+func (r *reader) ReadFrame() ([]byte, error) {
+        // Read the frame size.
+        count, err := binary.ReadUvarint(r)
+        if err != nil {
+                return nil, err
+        }
+
+        if count > uint64(r.maxSize) {
+                return nil, ErrFrameTooLarge
+        }
+
+        data := make([]byte, int(count))
+        if _, err := r.Read(data); err != nil {
+                return nil, err
+        }
+        return data, nil
+}
+
+// simpleByteReader implements the io.ByteReader interface for an io.Reader.
+type simpleByteReader struct {
+        io.Reader
+
+        buf [1]byte

[iannucci, 2015/08/03 22:21:24]

not threadsafe? why have it in the struct at all? You end up doing an allocation
for the slice anyway...

[dnj, 2015/08/03 22:42:58]

No, struct is not threadsafe :) Most readers aren't!

Since the array is static, it should be allocated as part of the struct
allocation.

[iannucci, 2015/08/04 17:13:50]

Right, but you have to slice it every time, which is an allocation. Might as
well just do a make in the function right?

+}
+
+func (r *simpleByteReader) ReadByte() (byte, error) {
+        _, err := r.Read(r.buf[:])
+        return r.buf[0], err
+}