Skip to content

Commit

Permalink
Reverts bc1239b, no longer needed to conform to legacy
Browse files Browse the repository at this point in the history
This fixes the tests and verifies that the fix from bc1239b, which
broke the tests, is no longer needed to conform to legacy compression.
evanphx committed Jan 8, 2024
1 parent 9542ba5 commit 6e17a24
Showing 3 changed files with 59 additions and 7 deletions.
5 changes: 1 addition & 4 deletions internal/lz4block/blocks.go
Original file line number Diff line number Diff line change
@@ -8,12 +8,9 @@ const (
Block256Kb
Block1Mb
Block4Mb
Block8Mb = 2 * Block4Mb
)

// In legacy mode all blocks are compressed regardless
// of the compressed size: use the bound size.
var Block8Mb = uint32(CompressBlockBound(8 << 20))

var (
BlockPool64K = sync.Pool{New: func() interface{} { return make([]byte, Block64Kb) }}
BlockPool256K = sync.Pool{New: func() interface{} { return make([]byte, Block256Kb) }}
4 changes: 1 addition & 3 deletions internal/lz4stream/block.go
Original file line number Diff line number Diff line change
@@ -224,9 +224,7 @@ func (b *FrameDataBlock) Close(f *Frame) {
func (b *FrameDataBlock) Compress(f *Frame, src []byte, level lz4block.CompressionLevel) *FrameDataBlock {
data := b.data
if f.isLegacy() {
// In legacy mode, the buffer is sized according to CompressBlockBound,
// but only 8Mb is buffered for compression.
src = src[:8<<20]
data = data[:cap(data)]
} else {
data = data[:len(src)] // trigger the incompressible flag in CompressBlock
}
57 changes: 57 additions & 0 deletions writer_test.go
Original file line number Diff line number Diff line change
@@ -7,6 +7,7 @@ import (
"io"
"io/ioutil"
"os"
"os/exec"
"reflect"
"strings"
"testing"
@@ -285,13 +286,69 @@ func TestWriterLegacy(t *testing.T) {
if _, err := io.Copy(out2, zr); err != nil {
t.Fatal(err)
}

if len(src) != out2.Len() {
t.Fatalf("uncompressed output not correct size. %d != %d", len(src), out2.Len())
}

if !bytes.Equal(out2.Bytes(), src) {
t.Fatal("uncompressed compressed output different from source")
}
})
}
}

func TestWriterLegacyCommand(t *testing.T) {
_, err := exec.LookPath("lz4")
if err != nil {
t.Skip("no lz4 binary to test against")
}

goldenFiles := []string{
"testdata/vmlinux_LZ4_19377",
"testdata/bzImage_lz4_isolated",
}

for _, fname := range goldenFiles {
t.Run(fname, func(t *testing.T) {
fname := fname
t.Parallel()

src, err := ioutil.ReadFile(fname)
if err != nil {
t.Fatal(err)
}

out := new(bytes.Buffer)
zw := lz4.NewWriter(out)
if err := zw.Apply(lz4.LegacyOption(true), lz4.CompressionLevelOption(lz4.Fast)); err != nil {
t.Fatal(err)
}
if _, err := io.Copy(zw, bytes.NewReader(src)); err != nil {
t.Fatal(err)
}
if err := zw.Close(); err != nil {
t.Fatal(err)
}

// write to filesystem for further checking
tmp, err := ioutil.TempFile("", "")
if err != nil {
t.Fatal(err)
}
defer os.Remove(tmp.Name())
if _, err := tmp.Write(out.Bytes()); err != nil {
t.Fatal(err)
}

cmd := exec.Command("lz4", "--test", tmp.Name())
if _, err := cmd.Output(); err != nil {
t.Fatal(err)
}
})
}
}

func TestWriterConcurrency(t *testing.T) {
const someGiantFile = "testdata/vmlinux_LZ4_19377"

0 comments on commit 6e17a24

Please sign in to comment.