tidwall
[BUG REPORT] Write call ignored if buffer is smaller than pgsize
Hi, I think I found a bug in the library.
I wrote a simple test that reads a file and writes to another, both using Pager's. Basically it just copies one file to another.
But for some reason the copied (written) file is some bytes smaller than the first file (the one I read data from).
I made some debugging and I think the cause is in the following code in pio func:
// Read contents of page from file for all read operations, and
// partial write operations. Ignore for full page writes.
if !write || pend-pstart < f.pgsize {
if err := f.read(p); err != nil {
return 0, err
}
}
Cuz when pend-pstart < f.pgsize, it will perform a read operation, regardless write = true.
I think it could be fixed with the following change:
// Read contents of page from file for all read operations.
// Ignore for all write operations.
if !write {
if err := f.read(p); err != nil {
return 0, err
}
}
Hi, That does look a little squirrelly. To fix it we'll need to reproduce the problem first. Ideally by adding a test that fails, and then apply the fix.
Ok, I will write a test and submit it.
FYI, I have ported the library to C code. I have been comparing the output of both versions.
I found the bug writting a file copy test for the c version, and when I compared both files, the copied file was 223 bytes smaller. So I debugged the code and found about the bug.
I wrote "I think I found a bug ..." cuz I have not written a go version yet to verify.
The fix works for the c version already.
I have updated the issue title to be more accurate of the problem.
And have confirmed that the issue does not happen in go.
Because in go calling 'f.file.ReadAt' does not return error, but in c 'read()' does return an error, probably an end of file error for trying to read a file when the offset is at the end of the file.
[UPDATE] I understand better now, ReatAt does not increase the file offset, that is why it does not return an EOF error.
Please, you can close the issue now. Sorry any inconvenience.
That’s really cool! Is the C version gonna public?
I am porting a music app for the nintendo 3ds. Once finished, I will submit the code to the app repo, and I hope It gets approved.
If not, I will upload it in my personal github page.
Hi there,
I am still doing some tests with the c port. And have found something interesting:
I have the following test:
func SmallBufferWrite() {
f1, err := os.OpenFile("test.bin", os.O_RDWR, 0644)
//f1, err := os.Create("test_.bin")
if err != nil {
os.Exit(-1)
}
p := NewPager(f1)
b := make([]byte, 256)
for i := range b {
b[i] = 0xb4
}
p.WriteAt(b, 0)
p.Flush()
f1.Close()
}
The file already exists, and I want to overwrite a part of it with a small buffer. In go the code works well, only 256 bytes of the file gets overwritten.
But in c it overwrites the whole page size.
How is that in go this code:
func (f *Pager) write(p *page) error {
off := p.num * f.pgsize
end := int64(f.pgsize)
if off+end > f.size {
end = f.size - off
}
_, err := f.file.WriteAt(p.data[:end], off)
if err != nil {
return err
}
return nil
}
Only writes 256 bytes and not the whole pgsize? When I print len(p.data[:end]) it shows 4096.
[UPDATE]
I understand now! The code I posted at the top, is used to "fill" the remaining data from the file, for partial writes. That made me think only 256 bytes were written.
Now, what happens if you open the file only for writing?
f1, err := os.OpenFile("test.bin", os.O_WRONLY, 0644)
It seems, the write is not done at all. Because it fails to read there:
if !write || pend-pstart < f.pgsize {
if err := f.read(p); err != nil {
return 0, err
}
}