added first unit tests for Frame

added Applier interface for cleaner Option handling

block.go

@@ -203,13 +203,13 @@ func CompressBlock(src, dst []byte, hashTable []int) (_ int, err error) {
 		hashTable[h] = si - 2
 	}
 
+lastLiterals:
 	if isNotCompressible && anchor == 0 {
 		// Incompressible.
 		return 0, nil
 	}
 
 	// Last literals.
-lastLiterals:
 	lLen := len(src) - anchor
 	if lLen < 0xF {
 		dst[di] = byte(lLen << 4)

frame.go

@@ -94,7 +94,7 @@ type FrameDescriptor struct {
 
 func (fd *FrameDescriptor) initW(_ *Writer) {
 	fd.Flags.VersionSet(1)
-	fd.Flags.BlockIndependenceSet(false)
+	fd.Flags.BlockIndependenceSet(true)
 }
 
 func (fd *FrameDescriptor) write(w *Writer) error {
@@ -106,15 +106,11 @@ func (fd *FrameDescriptor) write(w *Writer) error {
 	buf := w.buf[:2]
 	binary.LittleEndian.PutUint16(buf, uint16(fd.Flags))
 
-	var checksum uint32
 	if fd.Flags.Size() {
 		buf = buf[:10]
 		binary.LittleEndian.PutUint64(buf[2:], fd.ContentSize)
-		checksum = xxh32.ChecksumZero(buf)
-	} else {
-		checksum = xxh32.ChecksumZero(buf)
 	}
-	fd.Checksum = byte(checksum >> 8)
+	fd.Checksum = descriptorChecksum(buf)
 	buf = append(buf, fd.Checksum)
 
 	_, err := w.src.Write(buf)
@@ -122,42 +118,33 @@ func (fd *FrameDescriptor) write(w *Writer) error {
 }
 
 func (fd *FrameDescriptor) initR(r *Reader) error {
-	buf := r.buf[:2]
+	// Read the flags and the checksum, hoping that there is not content size.
+	buf := r.buf[:3]
 	if _, err := io.ReadFull(r.src, buf); err != nil {
 		return err
 	}
 	descr := binary.LittleEndian.Uint16(buf)
 	fd.Flags = DescriptorFlags(descr)
-
-	var checksum uint32
 	if fd.Flags.Size() {
+		// Append the 8 missing bytes.
 		buf = buf[:11]
-		if _, err := io.ReadFull(r.src, buf[2:]); err != nil {
-			return err
-		}
-		fd.ContentSize = binary.LittleEndian.Uint64(buf)
-		checksum = xxh32.ChecksumZero(buf)
-	} else {
-		buf = buf[:3]
-		var err error
-		if br, ok := r.src.(io.ByteReader); ok {
-			buf[2], err = br.ReadByte()
-		} else {
-			_, err = io.ReadFull(r.src, buf[2:])
-		}
-		if err != nil {
+		if _, err := io.ReadFull(r.src, buf[3:]); err != nil {
 			return err
 		}
-		checksum = xxh32.ChecksumZero(buf)
+		fd.ContentSize = binary.LittleEndian.Uint64(buf[2:])
 	}
-	fd.Checksum = buf[len(buf)-1]
-	if c := byte(checksum >> 8); fd.Checksum != c {
-		return fmt.Errorf("lz4: %w: got %x; expected %x", ErrInvalidHeaderChecksum, c, fd.Checksum)
+	fd.Checksum = buf[len(buf)-1] // the checksum is the last byte
+	buf = buf[:len(buf)-1]        // all descriptor fields except checksum
+	if c := descriptorChecksum(buf); fd.Checksum != c {
+		return fmt.Errorf("%w: got %x; expected %x", ErrInvalidHeaderChecksum, c, fd.Checksum)
 	}
-
 	return nil
 }
 
+func descriptorChecksum(buf []byte) byte {
+	return byte(xxh32.ChecksumZero(buf) >> 8)
+}
+
 type Blocks struct {
 	Block  *FrameDataBlock
 	Blocks chan chan *FrameDataBlock
@@ -295,6 +282,7 @@ func (b *FrameDataBlock) uncompress(r *Reader, dst []byte) (int, error) {
 	}
 	b.Size = DataBlockSize(x)
 	if b.Size == 0 {
+		// End of frame reached.
 		return 0, io.EOF
 	}
 
@@ -321,7 +309,7 @@ func (b *FrameDataBlock) uncompress(r *Reader, dst []byte) (int, error) {
 			return 0, err
 		}
 		if c := xxh32.ChecksumZero(data); c != b.Checksum {
-			return 0, fmt.Errorf("lz4: %w: got %x; expected %x", ErrInvalidBlockChecksum, c, b.Checksum)
+			return 0, fmt.Errorf("%w: got %x; expected %x", ErrInvalidBlockChecksum, c, b.Checksum)
 		}
 	}
 	if r.frame.Descriptor.Flags.ContentChecksum() {

frame_gen.go

@@ -5,32 +5,32 @@ package lz4
 // DescriptorFlags is defined as follow:
 //   field              bits
 //   -----              ----
-//   _                  4
-//   BlockSizeIndex     3
-//   _                  1
 //   _                  2
 //   ContentChecksum    1
 //   Size               1
 //   BlockChecksum      1
 //   BlockIndependence  1
 //   Version            2
+//   _                  4
+//   BlockSizeIndex     3
+//   _                  1
 type DescriptorFlags uint16
 
 // Getters.
-func (x DescriptorFlags) BlockSizeIndex() BlockSizeIndex { return BlockSizeIndex(x>>4&0x7) }
-func (x DescriptorFlags) ContentChecksum() bool          { return x>>10&1 != 0 }
-func (x DescriptorFlags) Size() bool                     { return x>>11&1 != 0 }
-func (x DescriptorFlags) BlockChecksum() bool            { return x>>12&1 != 0 }
-func (x DescriptorFlags) BlockIndependence() bool        { return x>>13&1 != 0 }
-func (x DescriptorFlags) Version() uint16                { return uint16(x>>14&0x3) }
+func (x DescriptorFlags) ContentChecksum() bool { return x>>2&1 != 0 }
+func (x DescriptorFlags) Size() bool { return x>>3&1 != 0 }
+func (x DescriptorFlags) BlockChecksum() bool { return x>>4&1 != 0 }
+func (x DescriptorFlags) BlockIndependence() bool { return x>>5&1 != 0 }
+func (x DescriptorFlags) Version() uint16 { return uint16(x>>6&0x3) }
+func (x DescriptorFlags) BlockSizeIndex() BlockSizeIndex { return BlockSizeIndex(x>>12&0x7) }
 
 // Setters.
-func (x *DescriptorFlags) BlockSizeIndexSet(v BlockSizeIndex) *DescriptorFlags { *x = *x&^(0x7<<4) | (DescriptorFlags(v)&0x7<<4); return x }
-func (x *DescriptorFlags) ContentChecksumSet(v bool) *DescriptorFlags { const b = 1<<10; if v { *x = *x&^b | b } else { *x &^= b }; return x }
-func (x *DescriptorFlags) SizeSet(v bool) *DescriptorFlags { const b = 1<<11; if v { *x = *x&^b | b } else { *x &^= b }; return x }
-func (x *DescriptorFlags) BlockChecksumSet(v bool) *DescriptorFlags { const b = 1<<12; if v { *x = *x&^b | b } else { *x &^= b }; return x }
-func (x *DescriptorFlags) BlockIndependenceSet(v bool) *DescriptorFlags { const b = 1<<13; if v { *x = *x&^b | b } else { *x &^= b }; return x }
-func (x *DescriptorFlags) VersionSet(v uint16) *DescriptorFlags { *x = *x&^(0x3<<14) | (DescriptorFlags(v)&0x3<<14); return x }
+func (x *DescriptorFlags) ContentChecksumSet(v bool) *DescriptorFlags { const b = 1<<2; if v { *x = *x&^b | b } else { *x &^= b }; return x }
+func (x *DescriptorFlags) SizeSet(v bool) *DescriptorFlags { const b = 1<<3; if v { *x = *x&^b | b } else { *x &^= b }; return x }
+func (x *DescriptorFlags) BlockChecksumSet(v bool) *DescriptorFlags { const b = 1<<4; if v { *x = *x&^b | b } else { *x &^= b }; return x }
+func (x *DescriptorFlags) BlockIndependenceSet(v bool) *DescriptorFlags { const b = 1<<5; if v { *x = *x&^b | b } else { *x &^= b }; return x }
+func (x *DescriptorFlags) VersionSet(v uint16) *DescriptorFlags { *x = *x&^(0x3<<6) | (DescriptorFlags(v)&0x3<<6); return x }
+func (x *DescriptorFlags) BlockSizeIndexSet(v BlockSizeIndex) *DescriptorFlags { *x = *x&^(0x7<<12) | (DescriptorFlags(v)&0x7<<12); return x }
 // Code generated by `gen.exe`. DO NOT EDIT.
 
 // DataBlockSize is defined as follow:
@@ -41,7 +41,7 @@ func (x *DescriptorFlags) VersionSet(v uint16) *DescriptorFlags { *x = *x&^(0x3<
 type DataBlockSize uint32
 
 // Getters.
-func (x DataBlockSize) size() int        { return int(x&0x7FFFFFFF) }
+func (x DataBlockSize) size() int { return int(x&0x7FFFFFFF) }
 func (x DataBlockSize) compressed() bool { return x>>31&1 != 0 }
 
 // Setters.

frame_test.go

@@ -0,0 +1,104 @@
+package lz4
+
+import (
+	"bytes"
+	"fmt"
+	"strings"
+	"testing"
+)
+
+func TestFrameDescriptor(t *testing.T) {
+	for _, tc := range []struct {
+		flags             string
+		bsum, csize, csum bool
+		size              uint64
+		bsize             BlockSize
+	}{
+		{"\x64\x40\xa7", false, false, true, 0, Block64Kb},
+		{"\x64\x50\x08", false, false, true, 0, Block256Kb},
+		{"\x64\x60\x85", false, false, true, 0, Block1Mb},
+		{"\x64\x70\xb9", false, false, true, 0, Block4Mb},
+	} {
+		s := tc.flags
+		label := fmt.Sprintf("%02x %02x %02x", s[0], s[1], s[2])
+		t.Run(label, func(t *testing.T) {
+			r := &Reader{src: strings.NewReader(tc.flags)}
+			var fd FrameDescriptor
+			if err := fd.initR(r); err != nil {
+				t.Fatal(err)
+			}
+
+			if got, want := fd.Flags.BlockChecksum(), tc.bsum; got != want {
+				t.Fatalf("got %v; want %v\n", got, want)
+			}
+			if got, want := fd.Flags.Size(), tc.csize; got != want {
+				t.Fatalf("got %v; want %v\n", got, want)
+			}
+			if got, want := fd.Flags.ContentChecksum(), tc.csum; got != want {
+				t.Fatalf("got %v; want %v\n", got, want)
+			}
+			if got, want := fd.ContentSize, tc.size; got != want {
+				t.Fatalf("got %v; want %v\n", got, want)
+			}
+			if got, want := fd.Flags.BlockSizeIndex(), tc.bsize.index(); got != want {
+				t.Fatalf("got %v; want %v\n", got, want)
+			}
+
+			buf := new(bytes.Buffer)
+			w := &Writer{src: buf}
+			fd.initW(nil)
+			fd.Checksum = 0
+			if err := fd.write(w); err != nil {
+				t.Fatal(err)
+			}
+			if got, want := buf.String(), tc.flags; got != want {
+				t.Fatalf("got %q; want %q\n", got, want)
+			}
+		})
+	}
+}
+
+func TestFrameDataBlock(t *testing.T) {
+	const sample = "abcd4566878dsvddddddqvq&&&&&((èdvshdvsvdsdh)"
+	min := func(a, b int) int {
+		if a < b {
+			return a
+		}
+		return b
+	}
+	for _, tc := range []struct {
+		data string
+		size BlockSize
+	}{
+		{"", Block64Kb},
+		{sample, Block64Kb},
+		{strings.Repeat(sample, 10), Block64Kb},
+	} {
+		label := fmt.Sprintf("%s (%d)", tc.data[:min(len(tc.data), 10)], len(tc.data))
+		t.Run(label, func(t *testing.T) {
+			data := tc.data
+			size := tc.size
+			zbuf := new(bytes.Buffer)
+			w := &Writer{src: zbuf, level: Fast}
+
+			block := newFrameDataBlock(size.index())
+			block.compress(w, []byte(data), nil)
+			if err := block.write(w); err != nil {
+				t.Fatal(err)
+			}
+
+			buf := make([]byte, size)
+			r := &Reader{src: zbuf}
+			n, err := block.uncompress(r, buf)
+			if err != nil {
+				t.Fatal(err)
+			}
+			if got, want := n, len(data); got != want {
+				t.Fatalf("got %d; want %d", got, want)
+			}
+			if got, want := string(buf[:n]), data; got != want {
+				t.Fatalf("got %q; want %q", got, want)
+			}
+		})
+	}
+}

gen.go

@@ -11,10 +11,6 @@ import (
 )
 
 type DescriptorFlags struct {
-	// BD
-	_              [4]int
-	BlockSizeIndex [3]lz4.BlockSizeIndex
-	_              [1]int
 	// FLG
 	_                 [2]int
 	ContentChecksum   [1]bool
@@ -22,6 +18,10 @@ type DescriptorFlags struct {
 	BlockChecksum     [1]bool
 	BlockIndependence [1]bool
 	Version           [2]uint16
+	// BD
+	_              [4]int
+	BlockSizeIndex [3]lz4.BlockSizeIndex
+	_              [1]int
 }
 
 type DataBlockSize struct {
@@ -36,7 +36,7 @@ func main() {
 	}
 	defer out.Close()
 
-	pkg := "v4"
+	pkg := "lz4"
 	for i, t := range []interface{}{
 		DescriptorFlags{}, DataBlockSize{},
 	} {

go.mod

@@ -5,6 +5,7 @@ go 1.14
 require (
 	code.cloudfoundry.org/bytefmt v0.0.0-20200131002437-cf55d5288a48
 	github.com/frankban/quicktest v1.9.0 // indirect
+	github.com/google/go-cmp v0.4.0
 	github.com/pierrec/cmdflag v0.0.2
 	github.com/pierrec/packer v0.0.0-20200419211718-decbba9fa6fa // indirect
 	github.com/schollz/progressbar v1.0.0

options.go

@@ -8,8 +8,14 @@ import (
 
 //go:generate go run golang.org/x/tools/cmd/stringer -type=BlockSize,CompressionLevel -output options_gen.go
 
-// Option defines the parameters to setup an LZ4 Writer or Reader.
-type Option func(*Reader, *Writer) error
+type (
+	Applier interface {
+		Apply(...Option) error
+		private()
+	}
+	// Option defines the parameters to setup an LZ4 Writer or Reader.
+	Option func(Applier) error
+)
 
 // Default options.
 var (
@@ -86,8 +92,9 @@ func (b BlockSizeIndex) put(buf []byte) {
 
 // BlockSizeOption defines the maximum size of compressed blocks (default=Block4Mb).
 func BlockSizeOption(size BlockSize) Option {
-	return func(r *Reader, w *Writer) error {
-		if r != nil {
+	return func(a Applier) error {
+		w, ok := a.(*Writer)
+		if !ok {
 			return ErrOptionNotApplicable
 		}
 		if !size.isValid() {
@@ -100,8 +107,9 @@ func BlockSizeOption(size BlockSize) Option {
 
 // BlockChecksumOption enables or disables block checksum (default=false).
 func BlockChecksumOption(flag bool) Option {
-	return func(r *Reader, w *Writer) error {
-		if r != nil {
+	return func(a Applier) error {
+		w, ok := a.(*Writer)
+		if !ok {
 			return ErrOptionNotApplicable
 		}
 		w.frame.Descriptor.Flags.BlockChecksumSet(flag)
@@ -111,8 +119,9 @@ func BlockChecksumOption(flag bool) Option {
 
 // ChecksumOption enables/disables all blocks checksum (default=true).
 func ChecksumOption(flag bool) Option {
-	return func(r *Reader, w *Writer) error {
-		if r != nil {
+	return func(a Applier) error {
+		w, ok := a.(*Writer)
+		if !ok {
 			return ErrOptionNotApplicable
 		}
 		w.frame.Descriptor.Flags.ContentChecksumSet(flag)
@@ -122,8 +131,9 @@ func ChecksumOption(flag bool) Option {
 
 // SizeOption sets the size of the original uncompressed data (default=0).
 func SizeOption(size uint64) Option {
-	return func(r *Reader, w *Writer) error {
-		if r != nil {
+	return func(a Applier) error {
+		w, ok := a.(*Writer)
+		if !ok {
 			return ErrOptionNotApplicable
 		}
 		w.frame.Descriptor.Flags.SizeSet(size > 0)
@@ -135,8 +145,9 @@ func SizeOption(size uint64) Option {
 // ConcurrencyOption sets the number of go routines used for compression.
 // If n<0, then the output of runtime.GOMAXPROCS(0) is used.
 func ConcurrencyOption(n int) Option {
-	return func(r *Reader, w *Writer) error {
-		if r != nil {
+	return func(a Applier) error {
+		w, ok := a.(*Writer)
+		if !ok {
 			return ErrOptionNotApplicable
 		}
 		switch n {
@@ -169,8 +180,9 @@ const (
 
 // CompressionLevelOption defines the compression level (default=Fast).
 func CompressionLevelOption(level CompressionLevel) Option {
-	return func(r *Reader, w *Writer) error {
-		if r != nil {
+	return func(a Applier) error {
+		w, ok := a.(*Writer)
+		if !ok {
 			return ErrOptionNotApplicable
 		}
 		switch level {
@@ -190,13 +202,15 @@ func OnBlockDoneOption(handler func(size int)) Option {
 	if handler == nil {
 		handler = onBlockDone
 	}
-	return func(r *Reader, w *Writer) error {
-		if r != nil {
+	return func(a Applier) error {
+		if r, ok := a.(*Reader); ok {
 			r.handler = handler
+			return nil
 		}
-		if w != nil {
+		if w, ok := a.(*Writer); ok {
 			w.handler = handler
+			return nil
 		}
-		return nil
+		return ErrOptionNotApplicable
 	}
 }

reader.go

@@ -17,7 +17,7 @@ var readerStates = []aState{
 func NewReader(r io.Reader) *Reader {
 	zr := new(Reader)
 	zr.state.init(readerStates)
-	_ = defaultOnBlockDone(zr, nil)
+	_ = zr.Apply(defaultOnBlockDone)
 	return zr.Reset(r)
 }
 
@@ -31,6 +31,8 @@ type Reader struct {
 	handler func(int)
 }
 
+func (*Reader) private() {}
+
 func (r *Reader) Apply(options ...Option) (err error) {
 	defer r.state.check(&err)
 	switch r.state.state {
@@ -41,7 +43,7 @@ func (r *Reader) Apply(options ...Option) (err error) {
 		return ErrCannotApplyOptions
 	}
 	for _, o := range options {
-		if err = o(r, nil); err != nil {
+		if err = o(r); err != nil {
 			return
 		}
 	}
@@ -79,19 +81,12 @@ func (r *Reader) Read(buf []byte) (n int, err error) {
 		return
 	}
 
+	var bn int
 	if r.idx > 0 {
 		// Some left over data, use it.
-		bn := copy(buf, r.data[r.idx:])
-		n += bn
-		r.idx += bn
-		if r.idx == len(r.data) {
-			// All data read, get ready for the next Read.
-			r.idx = 0
-		}
-		return
+		goto fillbuf
 	}
 	// No uncompressed data yet.
-	var bn int
 	for len(buf) >= len(r.data) {
 		// Input buffer large enough and no pending data: uncompress directly into it.
 		switch bn, err = r.frame.Blocks.Block.uncompress(r, buf); err {
@@ -113,11 +108,11 @@ func (r *Reader) Read(buf []byte) (n int, err error) {
 	switch bn, err = r.frame.Blocks.Block.uncompress(r, r.data); err {
 	case nil:
 		r.handler(bn)
-		n += bn
+		goto fillbuf
 	case io.EOF:
-		goto close
+	default:
+		return
 	}
-	return
 close:
 	r.handler(bn)
 	n += bn
@@ -125,6 +120,15 @@ close:
 	r.frame.Descriptor.Flags.BlockSizeIndex().put(r.data)
 	r.reset(nil)
 	return
+fillbuf:
+	bn = copy(buf, r.data[r.idx:])
+	n += bn
+	r.idx += bn
+	if r.idx == len(r.data) {
+		// All data read, get ready for the next Read.
+		r.idx = 0
+	}
+	return
 }
 
 func (r *Reader) reset(reader io.Reader) {

writer.go

@@ -15,10 +15,7 @@ var writerStates = []aState{
 func NewWriter(w io.Writer) *Writer {
 	zw := new(Writer)
 	zw.state.init(writerStates)
-	_ = defaultBlockSizeOption(nil, zw)
-	_ = defaultChecksumOption(nil, zw)
-	_ = defaultConcurrency(nil, zw)
-	_ = defaultOnBlockDone(nil, zw)
+	_ = zw.Apply(defaultBlockSizeOption, defaultChecksumOption, defaultConcurrency, defaultOnBlockDone)
 	return zw.Reset(w)
 }
 
@@ -35,6 +32,8 @@ type Writer struct {
 	handler func(int)
 }
 
+func (*Writer) private() {}
+
 func (w *Writer) Apply(options ...Option) (err error) {
 	defer w.state.check(&err)
 	switch w.state.state {
@@ -45,7 +44,7 @@ func (w *Writer) Apply(options ...Option) (err error) {
 		return ErrCannotApplyOptions
 	}
 	for _, o := range options {
-		if err = o(nil, w); err != nil {
+		if err = o(w); err != nil {
 			return
 		}
 	}