Skip to content

Fix memory allocation in countingReader #38

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fix memory allocation in countingReader #38

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 27 additions & 5 deletions cpio/cpio.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,12 +146,34 @@ func (cr *countingReader) Seek(offset int64, whence int) (int64, error) {
if offset == 0 {
return cr.curPos, nil
}
b := make([]byte, offset)
n, err := io.ReadFull(cr, b)
if err != nil && err != io.EOF {
return 0, err
totalRead := int64(0)
const chunkSize = int64(1024 * 1024)
buf := make([]byte, chunkSize)

remaining := offset
for remaining > 0 {
// Define chunk size to read
toRead := chunkSize
if remaining < chunkSize {
toRead = remaining
}

n, err := cr.Read(buf[:toRead])
totalRead += int64(n)
remaining -= int64(n)
if err != nil && err != io.EOF {
// If all was read, skip error
if totalRead >= offset {
err = nil
}
return 0, err
}

if err == io.EOF {
break
}
Comment on lines +149 to +174
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, thanks for your contribution! This is definitely worth fixing.

I believe this can be addressed much more simply by using io.Discard:

Suggested change
totalRead := int64(0)
const chunkSize = int64(1024 * 1024)
buf := make([]byte, chunkSize)
remaining := offset
for remaining > 0 {
// Define chunk size to read
toRead := chunkSize
if remaining < chunkSize {
toRead = remaining
}
n, err := cr.Read(buf[:toRead])
totalRead += int64(n)
remaining -= int64(n)
if err != nil && err != io.EOF {
// If all was read, skip error
if totalRead >= offset {
err = nil
}
return 0, err
}
if err == io.EOF {
break
}
return io.CopyN(io.Discard, cr, offset)

It may be a tiny bit less efficient but the simplicity is worth it in my opinion.

}
return int64(n), nil
return totalRead, nil
}

func pad(num int) int {
Expand Down