code review 162062: Robustness fixes for tar/archive.

Robustness fixes for tar/archive.

	  1. If all data is exhausted using Read then a following Next will
	     fail as if it saw EOF.

	     A test case for this was added.  It verifies the data using MD5.


	  2. Seeking isn't always possible (f.e. sockets and pipes).  Fallback
	     to read.

	     A test case for this was added.


	  3. Fix to readHeader (cleaner fix pointed out by rsc).

	     One of the test cases (TestReader) was modified to accommodate
	     this.


	  4. When Read has consumed all the data, don't try and read 0 bytes
	     from reader.

	     In cases where tr.nb is zero we attempt to read zero bytes and thus
	     never see an EOF (this is most easily seen when the 'tar source' is
	     something like bytes.Buffer{} as opposed to os.File).


          5. If write is used to the point of ErrWriteTooLong allow additional
	     file entries.


          6. Make close work as expected.  That is any further Write or
	     WriteHeader attempts will result in ErrWriteAfterClose.

[cw, 2009-11-30 10:19:45]

This obsoletes http://codereview.appspot.com/162047/show

[dsymonds, 2009-11-30 23:10:39]

http://codereview.appspot.com/162062/diff/1/2
File src/pkg/archive/tar/reader.go (right):

http://codereview.appspot.com/162062/diff/1/2#newcode100
src/pkg/archive/tar/reader.go:100: if err, ok := tr.err.(*os.PathError); ok &&
err.Error == os.ESPIPE {
This feels like too much special casing and having extra knowledge about the
type behind the Seeker interface, but I'll defer to rsc.

[rsc, 2009-11-30 23:15:56]

http://codereview.appspot.com/162062/diff/1/2
File src/pkg/archive/tar/reader.go (right):

http://codereview.appspot.com/162062/diff/1/2#newcode100
src/pkg/archive/tar/reader.go:100: if err, ok := tr.err.(*os.PathError); ok &&
err.Error == os.ESPIPE {
On 2009/11/30 23:10:40, dsymonds1 wrote:
> This feels like too much special casing and having extra knowledge about the
> type behind the Seeker interface, but I'll defer to rsc.

I think I agree.  Also this can be rearranged to get
rid of the flag, by moving the final assignment up:

nr := ...
tr.nb, tr.pad = 0, 0;
if sr, ok := tr.r.(io.Seeker); ok {
    if _, err := sr.Seek(nr, 1); err == nil {
        return;
    }
}
_, tr.err = io.Copyn....

http://codereview.appspot.com/162062/diff/1/2#newcode225
src/pkg/archive/tar/reader.go:225: // EOF isn't an error condition; don't pass
up to the
What breaks if we don't have this condition?
It seems like it would be fine to set tr.err.

[cw, 2009-12-01 05:52:30]

Updated comments.  Code to follow.

http://codereview.appspot.com/162062/diff/1/2
File src/pkg/archive/tar/reader.go (right):

http://codereview.appspot.com/162062/diff/1/2#newcode100
src/pkg/archive/tar/reader.go:100: if err, ok := tr.err.(*os.PathError); ok &&
err.Error == os.ESPIPE {
On 2009/11/30 23:15:56, rsc wrote:
> On 2009/11/30 23:10:40, dsymonds1 wrote:
> > This feels like too much special casing and having extra knowledge about the
> > type behind the Seeker interface, but I'll defer to rsc.
> 
> I think I agree.  Also this can be rearranged to get
> rid of the flag, by moving the final assignment up:
> 
> nr := ...
> tr.nb, tr.pad = 0, 0;
> if sr, ok := tr.r.(io.Seeker); ok {
>     if _, err := sr.Seek(nr, 1); err == nil {
>         return;
>     }
> }
> _, tr.err = io.Copyn....

I actually didn't do this (type assertion & check) at first but changed it after
seeing some other code doing something similar and felt it was 'safer' to only
do this on seek failures, not all errors; the idea being that if seek failed
with EBADF or EIO or something we could avoid the read/copy which would also
fail

that said... it's a pretty pointless optimization so not that important, and if
we are to care much about this optimization we would have a flag in 'tr' that we
would set to avoid trying to seek for subsequent reads

i'm not super picky either way and i like the rearranged version, i'll retest &
resubmit

http://codereview.appspot.com/162062/diff/1/2#newcode225
src/pkg/archive/tar/reader.go:225: // EOF isn't an error condition; don't pass
up to the
On 2009/11/30 23:15:56, rsc wrote:
> What breaks if we don't have this condition?

subsequent .Next()'s after .Read() till EOF

> It seems like it would be fine to set tr.err.

tr.err == os.EOF means the tarreader is at EOF so .Next will never find another
file entry

the issue is that if you do:


.Next()  // get first header

for { .Read() ; if EOF break // consume FILE entry until EOF }

.Next() // <- never finds anything, tr.err is at EOF even though there are more
headers left to read


The test case does actually check for this behavior

i'll change the comment to try and make this clearer

[cw, 2009-12-01 05:57:56]

Hello rsc, dsymonds1 (cc: cw),

I'd like you to review the following change.

[dsymonds, 2009-12-01 06:03:17]

LGTM

http://codereview.appspot.com/162062/diff/3005/2007
File src/pkg/archive/tar/reader.go (right):

http://codereview.appspot.com/162062/diff/3005/2007#newcode104
src/pkg/archive/tar/reader.go:104: tr.nb, tr.pad = 0, 0;
delete this line

http://codereview.appspot.com/162062/diff/3005/2007#newcode207
src/pkg/archive/tar/reader.go:207: // It returns 0, nil when it reaches the end
of that entry,
This comment is out of date now.

[cw, 2009-12-01 06:05:16]

more comments

http://codereview.appspot.com/162062/diff/3005/2007
File src/pkg/archive/tar/reader.go (right):

http://codereview.appspot.com/162062/diff/3005/2007#newcode104
src/pkg/archive/tar/reader.go:104: tr.nb, tr.pad = 0, 0;
On 2009/12/01 06:03:17, dsymonds1 wrote:
> delete this line

yes, of course

http://codereview.appspot.com/162062/diff/3005/2007#newcode207
src/pkg/archive/tar/reader.go:207: // It returns 0, nil when it reaches the end
of that entry,
On 2009/12/01 06:03:17, dsymonds1 wrote:
> This comment is out of date now.

will yank

[cw, 2009-12-01 06:09:08]

Hello rsc, dsymonds1,

I'd like you to review the following change.

[rsc, 2009-12-01 08:31:14]

> .Next()  // get first header
>
> for { .Read() ; if EOF break // consume FILE entry until EOF }
>
> .Next() // <- never finds anything, tr.err is at EOF even though there
> are more headers left to read

i still don't understand but i haven't looked
at the tests - i will do that tomorrow.
it seems to me that if Read gets to EOF,
it's okay for Next to return EOF - we got
to EOF, so there is no next file.

russ

[cw, 2009-12-01 10:00:29]

On Tue, Dec 01, 2009 at 12:31:07AM -0800, Russ Cox wrote:

> i still don't understand but i haven't looked
> at the tests - i will do that tomorrow.
> it seems to me that if Read gets to EOF,
> it's okay for Next to return EOF - we got
> to EOF, so there is no next file.

Read is just for the file entry not the tarreader, it can get EOF
before the tarreader as it's only part of the tarreaders stream

[cw, 2009-12-02 01:08:30]

What's the status of this?

[rsc, 2009-12-03 21:05:52]

http://codereview.appspot.com/162062/diff/3012/2014
File src/pkg/archive/tar/reader.go (right):

http://codereview.appspot.com/162062/diff/3012/2014#newcode204
src/pkg/archive/tar/reader.go:204: // Read(b) reads from the current entry in
the tar archive.  It
drop (b); not the local style.
put line breaks back the way they were,
so that text is easier to scan.

http://codereview.appspot.com/162062/diff/3012/2014#newcode218
src/pkg/archive/tar/reader.go:218: if err != os.EOF {
I still don't understand this.
I think the correct code is:

if err == os.EOF && tr.nb > 0 {
    err = io.ErrUnexpectedEOF;
}
tr.err = err;

If you get EOF at the right time,
i.e. when tr.nb == 0, then I think it's
okay to record EOF: you've reached the
end of the whole tar archive and there is
no point to continuing to read.

What am I missing?

[rsc, 2009-12-04 19:02:20]

cw - waiting on a reply


On Thu, Dec 3, 2009 at 13:05,  <rsc@golang.org> wrote:
>
> http://codereview.appspot.com/162062/diff/3012/2014
> File src/pkg/archive/tar/reader.go (right):
>
> http://codereview.appspot.com/162062/diff/3012/2014#newcode204
> src/pkg/archive/tar/reader.go:204: // Read(b) reads from the current
> entry in the tar archive.  It
> drop (b); not the local style.
> put line breaks back the way they were,
> so that text is easier to scan.
>
> http://codereview.appspot.com/162062/diff/3012/2014#newcode218
> src/pkg/archive/tar/reader.go:218: if err != os.EOF {
> I still don't understand this.
> I think the correct code is:
>
> if err == os.EOF && tr.nb > 0 {
>    err = io.ErrUnexpectedEOF;
> }
> tr.err = err;
>
> If you get EOF at the right time,
> i.e. when tr.nb == 0, then I think it's
> okay to record EOF: you've reached the
> end of the whole tar archive and there is
> no point to continuing to read.
>
> What am I missing?
>
> http://codereview.appspot.com/162062
>

[cw, 2009-12-07 10:03:55]

[Updated]

Hi,

I'd like you to review the following change.

http://codereview.appspot.com/162062/diff/3012/2014
File src/pkg/archive/tar/reader.go (right):

http://codereview.appspot.com/162062/diff/3012/2014#newcode204
src/pkg/archive/tar/reader.go:204: // Read(b) reads from the current entry in
the tar archive.  It
On 2009/12/03 21:05:52, rsc wrote:
> drop (b); not the local style.
> put line breaks back the way they were,
> so that text is easier to scan.

np (it was the result of emacs M-q wrapping)

http://codereview.appspot.com/162062/diff/3012/2014#newcode218
src/pkg/archive/tar/reader.go:218: if err != os.EOF {
On 2009/12/03 21:05:52, rsc wrote:

> I think the correct code is:
> 
> if err == os.EOF && tr.nb > 0 {
>     err = io.ErrUnexpectedEOF;
> }
> tr.err = err;

Yes.  I agree.  The reason the code was different originally was because the
first version of this was missing a hunk now present.

[rsc, 2009-12-07 20:54:48]

http://codereview.appspot.com/162062/diff/5001/5002
File src/pkg/archive/tar/reader.go (right):

http://codereview.appspot.com/162062/diff/5001/5002#newcode209
src/pkg/archive/tar/reader.go:209: // file entry completely consumed
s/entry completely //

http://codereview.appspot.com/162062/diff/5001/5004
File src/pkg/archive/tar/writer.go (right):

http://codereview.appspot.com/162062/diff/5001/5004#newcode110
src/pkg/archive/tar/writer.go:110: // WriteHeader calls Flush if it is not the
first header.  Calling
Start a new line, making the text easier to scan.
Avoid "this"

// If not enough bytes of the current file have been 
// written, WriteHeader will pad it with zeros to match
// the length promised in the previous header.

the part about writing after close doesn't need to be spelled out.

http://codereview.appspot.com/162062/diff/5001/5004#newcode115
src/pkg/archive/tar/writer.go:115: return os.EOF;
EINVAL

http://codereview.appspot.com/162062/diff/5001/5004#newcode174
src/pkg/archive/tar/writer.go:174: err = os.EOF;
s/EOF/EINVAL/

or make a new ErrWriteAfterClose

[cw, 2009-12-07 23:38:43]

Hello rsc, dsymonds1,

I'd like you to review the following change.

http://codereview.appspot.com/162062/diff/5001/5002
File src/pkg/archive/tar/reader.go (right):

http://codereview.appspot.com/162062/diff/5001/5002#newcode209
src/pkg/archive/tar/reader.go:209: // file entry completely consumed
> s/entry completely //

done

http://codereview.appspot.com/162062/diff/5001/5004
File src/pkg/archive/tar/writer.go (right):

http://codereview.appspot.com/162062/diff/5001/5004#newcode110
src/pkg/archive/tar/writer.go:110: // WriteHeader calls Flush if it is not the
first header.  Calling
> Start a new line, making the text easier to scan.
> Avoid "this"
> 
> // If not enough bytes of the current file have been 
> // written, WriteHeader will pad it with zeros to match
> // the length promised in the previous header.
> 
> the part about writing after close doesn't need to be spelled out.

done

http://codereview.appspot.com/162062/diff/5001/5004#newcode115
src/pkg/archive/tar/writer.go:115: return os.EOF;
> EINVAL

done

[rsc, 2009-12-11 20:48:31]

looks good; two small things

http://codereview.appspot.com/162062/diff/5011/6004
File src/pkg/archive/tar/reader.go (right):

http://codereview.appspot.com/162062/diff/5011/6004#newcode128
src/pkg/archive/tar/reader.go:128: }
} else { 
    tr.err = HeaderError;  // zero block and then non-zero block
}

http://codereview.appspot.com/162062/diff/5011/6006
File src/pkg/archive/tar/writer.go (right):

http://codereview.appspot.com/162062/diff/5011/6006#newcode171
src/pkg/archive/tar/writer.go:171: // after Close will return ErrWriteTooLong.
drop this sentence - it's wrong and can be left unsaid.

http://codereview.appspot.com/162062/diff/5011/6006#newcode192
src/pkg/archive/tar/writer.go:192: // Close finishes writing to the tar archive.
 Trailing blocks are
// Close closes the tar archive, flushing any unwritten
// data to the underlying writer. 

the rest is implied by the usual meaning of close

[cw, 2009-12-11 21:49:36]

Hello rsc, dsymonds1 (cc: cw),

I'd like you to review the following change.

[rsc, 2009-12-11 21:53:24]

tried to apply your change but it doesn't work for me.

; gotest
rm -f _test/archive/tar.a _gotest_.8
8g -o _gotest_.8 common.go reader.go writer.go    reader_test.go writer_test.go
rm -f _test/archive/tar.a
gopack grc _test/archive/tar.a _gotest_.8 
--- FAIL: tar.TestNonSeekable
	Didn't process all files
	expected: 2
	processed 1
FAIL

[cw, 2009-12-11 22:55:36]

> --- FAIL: tar.TestNonSeekable
> 	Didn't process all files
> 	expected: 2
> 	processed 1
> FAIL

bleh; let me look at that, i was sure this worked before i did an upload

[cw, 2009-12-11 23:02:40]

I can't reproduce this failure.  Arch/platform?  Dependant?

cw@lysdexia:~/wk/go/go.hg/src/pkg/archive/tar$ gotest
rm -f _test/archive/tar.a _gotest_.6
6g -o _gotest_.6 common.go reader.go writer.go    reader_test.go writer_test.go
rm -f _test/archive/tar.a
gopack grc _test/archive/tar.a _gotest_.6
PASS

Should I be doing something else?

tip:

 changeset:   4390:71447bfa8282
 tag:         tip
 user:        Russ Cox <rsc@golang.org>
 date:        Fri Dec 11 13:54:00 2009 -0800


Might you have some other change which is affecting this?

[rsc, 2009-12-11 23:07:17]

Only fails on darwin/386.  darwin/amd64 and linux/* seem to work.

; dtruss 8.out
--- FAIL: tar.TestNonSeekable
	Didn't process all files
	expected: 2
	processed 1
SYSCALL(args) 		 = return
getpid(0x0, 0x0, 0x0)		 = 55994 0
__sysctl(0xBFFFE588, 0x3, 0xBFFFF9A8)		 = 0 0
__sysctl(0xBFFFE26C, 0x2, 0xBFFFE274)		 = 0 0
bsdthread_register(0x227FF0, 0x2602A4, 0x1000)		 = 0 0
open_nocancel("/dev/urandom\0", 0x0, 0x0)		 = 3 0
read_nocancel(0x3,
"\364\347\340\362\3652l\205\336`\330!|\036\354\tO\373\325\220y\3353\354\024\001*m\343_\003\374\0",
0x20)		 = 32 0
close_nocancel(0x3)		 = 0 0
mmap(0x0, 0x3000, 0x3, 0x1002, 0x1000000, 0x0)		 = 0x92000 0
mmap(0x0, 0x200000, 0x3, 0x1002, 0x7000000, 0x0)		 = 0x3EC000 0
munmap(0x3EC000, 0x14000)		 = 0 0
munmap(0x500000, 0xEC000)		 = 0 0
mmap(0x0, 0x3000, 0x3, 0x1002, 0x1000000, 0x0)		 = 0x95000 0
getpid(0x0, 0x3000, 0x3)		 = 55994 0
lseek(0x3, 0x200, 0x1)		 = 1024 0
read(0x3, "small2.txt\0", 0x200)		 = 512 0
lseek(0x3, 0x200, 0x1)		 = 2048 0
read(0x3, "\0", 0x200)		 = 512 0
read(0x3, "\0", 0x200)		 = 512 0
close(0x4)		 = 0 0
write(0x1, "--- FAIL:\0", 0x9)		 = 9 0
write(0x1, " \0", 0x1)		 = 1 0
write(0x1, "tar.TestNonSeekable\0", 0x13)		 = 19 0
write(0x1, "\n\0", 0x1)		 = 1 0
write(0x1, "\tDidn't process all files\n\texpected: 2\n\tprocessed
1\n\0", 0x34)		 = 52 0
dtrace: error on enabled probe ID 1741 (ID 17725:
syscall::read:return): invalid address (0x53a000) in action #12 at DIF
offset 52
write(0xFFFFFFFF, "\0", 0x0)		 = -1 Err#9
dtrace: error on enabled probe ID 1741 (ID 17725:
syscall::read:return): invalid address (0x53a000) in action #12 at DIF
offset 52
write(0xFFFFFFFF, "\0", 0x0)		 = -1 Err#9
dtrace: error on enabled probe ID 1741 (ID 17725:
syscall::read:return): invalid address (0x53a000) in action #12 at DIF
offset 52
write(0xFFFFFFFF, "\0", 0x0)		 = -1 Err#9
dtrace: error on enabled probe ID 1741 (ID 17725:
syscall::read:return): invalid address (0x53a000) in action #12 at DIF
offset 52
write(0xFFFFFFFF, "\0", 0x0)		 = -1 Err#9
dtrace: error on enabled probe ID 1741 (ID 17725:
syscall::read:return): invalid address (0x53a000) in action #12 at DIF
offset 52
write(0xFFFFFFFF, "\0", 0x0)		 = -1 Err#9
dtrace: error on enabled probe ID 1741 (ID 17725:
syscall::read:return): invalid address (0x53a000) in action #12 at DIF
offset 52
write(0xFFFFFFFF, "\0", 0x0)		 = -1 Err#9
dtrace: error on enabled probe ID 1741 (ID 17725:
syscall::read:return): invalid address (0x53a000) in action #12 at DIF
offset 52
write(0xFFFFFFFF, "\0", 0x0)		 = -1 Err#9
dtrace: error on enabled probe ID 1741 (ID 17725:
syscall::read:return): invalid address (0x53a000) in action #12 at DIF
offset 52
write(0xFFFFFFFF, "\0", 0x0)		 = -1 Err#9
dtrace: error on enabled probe ID 1741 (ID 17725:
syscall::read:return): invalid address (0x53a000) in action #12 at DIF
offset 52
write(0xFFFFFFFF, "\0", 0x0)		 = -1 Err#9
dtrace: error on enabled probe ID 1741 (ID 17725:
syscall::read:return): invalid address (0x53a000) in action #12 at DIF
offset 52
write(0xFFFFFFFF, "\0", 0x0)		 = -1 Err#9
dtrace: error on enabled probe ID 1741 (ID 17725:
syscall::read:return): invalid address (0x53a000) in action #12 at DIF
offset 52
write(0xFFFFFFFF, "\0", 0x0)		 = -1 Err#9
dtrace: error on enabled probe ID 1741 (ID 17725:
syscall::read:return): invalid address (0x53a000) in action #12 at DIF
offset 52
FAILwrite(0xFFFFFFFF, "\0", 0x0)		 = -1 Err#9

dtrace: error on enabled probe ID 1741 (ID 17725:
syscall::read:return): invalid address (0x53a000) in action #12 at DIF
offset 52
write(0xFFFFFFFF, "\0", 0x0)		 = -1 Err#9
dtrace: error on enabled probe ID 1741 (ID 17725:
syscall::read:return): invalid address (0x53a000) in action #12 at DIF
offset 52
write(0xFFFFFFFF, "\0", 0x0)		 = -1 Err#9
dtrace: error on enabled probe ID 1741 (ID 17725:
syscall::read:return): invalid address (0x53a000) in action #12 at DIF
offset 52
write(0xFFFFFFFF, "\0", 0x0)		 = -1 Err#9
dtrace: error on enabled probe ID 1741 (ID 17725:
syscall::read:return): invalid address (0x53a000) in action #12 at DIF
offset 52
open_nocancel(".\0", 0x0, 0x0)		 = 3 0
fstat64(0x3, 0xBFFFE304, 0x0)		 = 0 0
fcntl_nocancel(0x3, 0x32, 0xFFFFFFFFBFFFF588)		 = 0 0
close_nocancel(0x3)		 = 0 0
stat64("/home/rsc/go/src/pkg/archive/tar\0", 0xBFFFE298,
0xFFFFFFFFBFFFF588)		 = 0 0
issetugid(0xBFFFF588, 0xBFFFE298, 0xFFFFFFFFBFFFF588)		 = 0 0
stat("/usr/lib/dtrace/libdtrace_dyld.dylib\0", 0xBFFFD5F8,
0xFFFFFFFFBFFFF588)		 = 0 0
open("/usr/lib/dtrace/libdtrace_dyld.dylib\0", 0x0, 0x0)		 = 3 0
pread(0x3, "\312\376\272\276\0", 0x1000, 0x0)		 = 4096 0
pread(0x3, "\316\372\355\376\a\0", 0x1000, 0x9000)		 = 4096 0
mmap(0x78000, 0x1000, 0x5, 0x12, 0x3, 0x100000000)		 = 0x78000 0
mmap(0x79000, 0x1000, 0x3, 0x12, 0x3, 0x100000000)		 = 0x79000 0
mmap(0x7A000, 0x1000, 0x7, 0x12, 0x3, 0x100000000)		 = 0x7A000 0
mmap(0x7B000, 0x1900, 0x1, 0x12, 0x3, 0x100000000)		 = 0x7B000 0
fcntl(0x3, 0x2C, 0xFFFFFFFFBFFFBC64)		 = 0 0
close(0x3)		 = 0 0
stat("/usr/lib/libgcc_s.1.dylib\0", 0xBFFFD3F8, 0xFFFFFFFFBFFFBC64)		 = 0 0
open("/usr/lib/libgcc_s.1.dylib\0", 0x0, 0x0)		 = 3 0
pread(0x3, "\312\376\272\276\0", 0x1000, 0x0)		 = 4096 0
pread(0x3, "\316\372\355\376\a\0", 0x1000, 0x1000)		 = 4096 0
mmap(0x7D000, 0x8000, 0x5, 0x12, 0x3, 0x100000000)		 = 0x7D000 0
mmap(0x85000, 0x1000, 0x3, 0x12, 0x3, 0x100000000)		 = 0x85000 0
mmap(0x86000, 0x1000, 0x7, 0x12, 0x3, 0x100000000)		 = 0x86000 0
mmap(0x87000, 0x2530, 0x1, 0x12, 0x3, 0x100000000)		 = 0x87000 0
fcntl(0x3, 0x2C, 0xFFFFFFFFBFFFB9E4)		 = 0 0
close(0x3)		 = 0 0
stat("/usr/lib/libSystem.B.dylib\0", 0xBFFFD3F8, 0xFFFFFFFFBFFFB9E4)		 = 0 0
open("/usr/lib/libSystem.B.dylib\0", 0x0, 0x0)		 = 3 0
pread(0x3, "\312\376\272\276\0", 0x1000, 0x0)		 = 4096 0
pread(0x3, "\316\372\355\376\a\0", 0x1000, 0x3E4000)		 = 4096 0
mmap(0x1F6000, 0x168000, 0x5, 0x12, 0x3, 0x100000000)		 = 0x1F6000 0
mmap(0x35E000, 0xA000, 0x3, 0x12, 0x3, 0x100000000)		 = 0x35E000 0
mmap(0x39D000, 0x2000, 0x7, 0x12, 0x3, 0x100000000)		 = 0x39D000 0
mmap(0x39F000, 0x4CDF0, 0x1, 0x12, 0x3, 0x100000000)		 = 0x39F000 0
fcntl(0x3, 0x2C, 0xFFFFFFFFBFFFB834)		 = 0 0
close(0x3)		 = 0 0
stat("/usr/lib/system/libmathCommon.A.dylib\0", 0xBFFFD238,
0xFFFFFFFFBFFFB834)		 = 0 0
open("/usr/lib/system/libmathCommon.A.dylib\0", 0x0, 0x0)		 = 3 0
pread(0x3, "\312\376\272\276\0", 0x1000, 0x0)		 = 4096 0
pread(0x3, "\316\372\355\376\a\0", 0x1000, 0x13000)		 = 4096 0
mmap(0x8A000, 0x5000, 0x5, 0x12, 0x3, 0x100000000)		 = 0x8A000 0
mmap(0x8F000, 0x1000, 0x3, 0x12, 0x3, 0x100000000)		 = 0x8F000 0
mmap(0x90000, 0x1C90, 0x1, 0x12, 0x3, 0x100000000)		 = 0x90000 0
fcntl(0x3, 0x2C, 0xFFFFFFFFBFFFBA34)		 = 0 0
close(0x3)		 = 0 0
open("/dev/dtracehelper\0", 0x2, 0xBFFFE414)		 = 3 0
ioctl(0x3, 0x80086804, 0xBFFFE398)		 = 0 0
close(0x3)		 = 0 0
sigaction(0x3, 0x75854, 0x0)		 = 0 0
sigaction(0x4, 0x75854, 0x0)		 = 0 0
sigaction(0x5, 0x75854, 0x0)		 = 0 0
sigaction(0x6, 0x75854, 0x0)		 = 0 0
sigaction(0x7, 0x75854, 0x0)		 = 0 0
sigaction(0x8, 0x75854, 0x0)		 = 0 0
sigaction(0xA, 0x75854, 0x0)		 = 0 0
sigaction(0xB, 0x75854, 0x0)		 = 0 0
sigaction(0xC, 0x75854, 0x0)		 = 0 0
sigaction(0xD, 0x75854, 0x0)		 = 0 0
sigaction(0x14, 0x75854, 0x0)		 = 0 0
sigaction(0x1C, 0x75854, 0x0)		 = 0 0
bsdthread_register(0x310F1, 0x0, 0x0)		 = 0 0
mmap(0x0, 0x20000, 0x7, 0x1002, 0xFFFFFFFF, 0x100000000)		 = 0x98000 0
mmap(0x0, 0x100000, 0x7, 0x1002, 0xFFFFFFFF, 0x100000000)		 = 0x500000 0
mmap(0x0, 0x1000, 0x7, 0x1002, 0xFFFFFFFF, 0x100000000)		 = 0xB8000 0
mmap(0x0, 0x20000, 0x7, 0x1002, 0xFFFFFFFF, 0x100000000)		 = 0xB9000 0
sigaltstack(0xBFFFFA04, 0x0, 0x7)		 = 0 0
mmap(0x0, 0x20000, 0x7, 0x1002, 0xFFFFFFFF, 0x100000000)		 = 0xD9000 0
open("testdata/gnu.tar\0", 0x0, 0x124)		 = 3 0
fcntl(0x3, 0x2, 0x1)		 = 0 0
lseek(0x3, 0x0, 0x1)		 = 0 0
read(0x3, "small.txt\0", 0x200)		 = 512 0
open("testdata/gnu.tar\0", 0x0, 0x124)		 = 4 0
fcntl(0x4, 0x2, 0x1)		 = 0 0
lseek(0x4, 0x0, 0x1)		 = 0 0
read(0x4, "small.txt\0", 0x200)		 = 512 0
read(0x4, "Kilt\0", 0x4)		 = 4 0
lseek(0x4, 0x1FC, 0x1)		 = 1024 0
read(0x4, "small2.txt\0", 0x200)		 = 512 0
read(0x4, "Google\0", 0x6)		 = 6 0
close(0x4)		 = 0 0
open("testdata/gnu.tar\0", 0x0, 0x124)		 = 4 0
fcntl(0x4, 0x2, 0x1)		 = 0 0
lseek(0x4, 0x0, 0x1)		 = 0 0
read(0x4, "small.txt\0", 0x200)		 = 512 0
read(0x4, "Kilts\0", 0x5)		 = 5 0
lseek(0x4, 0x1FB, 0x1)		 = 1024 0
read(0x4, "small2.txt\0", 0x200)		 = 512 0
read(0x4, "Google.c\0", 0x8)		 = 8 0
read(0x4, "om\n\0", 0x3)		 = 3 0
lseek(0x4, 0x1F5, 0x1)		 = 2048 0
read(0x4, "\0", 0x200)		 = 512 0
read(0x4, "\0", 0x200)		 = 512 0
close(0x4)		 = 0 0
open("testdata/gnu.tar\0", 0x0, 0x124)		 = 4 0
fcntl(0x4, 0x2, 0x1)		 = 0 0
pipe(0x4, 0x2, 0x1)		 = 5 0
fcntl(0x5, 0x2, 0x1)		 = 0 0
fcntl(0x6, 0x2, 0x1)		 = 0 0
bsdthread_create(0x2AB4D, 0x5250A0, 0x52C100)		 = 5272832 0
lseek(0x5, 0x0, 0x1)		 = -1 Err#29
read(0x5, "small.txt\0", 0x200)		 = 512 0
lseek(0x5, 0x200, 0x1)		 = -1 Err#29
read(0x5, "Kilts\0", 0x200)		 = 512 0
open("testdata/writer.tar\0", 0x0, 0x0)		 = 4 0
fcntl(0x4, 0x2, 0x1)		 = 0 0
fstat64(0x4, 0x524E00, 0x1)		 = 0 0
read(0x4, "small.txt\0", 0x200)		 = 512 0
read(0x4, "Kilts\0", 0x400)		 = 1024 0
read(0x4, "Google.com\n\0", 0x800)		 = 1536 0
read(0x4, "\0", 0x200)		 = 0 0
close(0x4)		 = 0 0
open("testdata/writer-big.tar\0", 0x0, 0x0)		 = 4 0
write(0xFFFFFFFF, "\0", 0x0)		 = -1 Err#9
dtrace: error on enabled probe ID 1741 (ID 17725:
syscall::read:return): invalid address (0x53a000) in action #12 at DIF
offset 52
dtrace: error on enabled probe ID 1741 (ID 17725:
syscall::read:return): invalid address (0x53a000) in action #12 at DIF
offset 52
write(0xFFFFFFFF, "\0", 0x0)		 = -1 Err#9
dtrace: error on enabled probe ID 1741 (ID 17725:
syscall::read:return): invalid address (0x53a000) in action #12 at DIF
offset 52
write(0xFFFFFFFF, "\0", 0x0)		 = -1 Err#9
dtrace: error on enabled probe ID 1741 (ID 17725:
syscall::read:return): invalid address (0x53a000) in action #12 at DIF
offset 52
write(0x1, "FAIL\0", 0x4)		 = 4 0
write(0x1, "\n\0", 0x1)		 = 1 0
write(0xFFFFFFFF, "\0", 0x0)		 = -1 Err#9
sigaltstack(0x52C0C4, 0x0, 0x0)		 = 0 0
dtrace: error on enabled probe ID 1741 (ID 17725:
syscall::read:return): invalid address (0x53a000) in action #12 at DIF
offset 52
write(0x6, "small.txt\0", 0xC00)		 = 3072 0
dtrace: error on enabled probe ID 1741 (ID 17725:
syscall::read:return): invalid address (0x53a000) in action #12 at DIF
offset 52
write(0x6, "\0", 0x0)		 = 0 0
close(0x6)		 = 0 0
dtrace: error on enabled probe ID 1741 (ID 17725:
syscall::read:return): invalid address (0x53a000) in action #12 at DIF
offset 52
write(0xFFFFFFFF, "\0", 0x0)		 = -1 Err#9
dtrace: error on enabled probe ID 1741 (ID 17725:
syscall::read:return): invalid address (0x53a000) in action #12 at DIF
offset 52
write(0xFFFFFFFF, "\0", 0x0)		 = -1 Err#9
dtrace: error on enabled probe ID 1741 (ID 17725:
syscall::read:return): invalid address (0x53a000) in action #12 at DIF
offset 52
write(0xFFFFFFFF, "\0", 0x0)		 = -1 Err#9
dtrace: error on enabled probe ID 1741 (ID 17725:
syscall::read:return): invalid address (0x53a000) in action #12 at DIF
offset 52
write(0xFFFFFFFF, "\0", 0x0)		 = -1 Err#9
dtrace: error on enabled probe ID 1741 (ID 17725:
syscall::read:return): invalid address (0x53a000) in action #12 at DIF
offset 52
write(0xFFFFFFFF, "\0", 0x0)		 = -1 Err#9
dtrace: error on enabled probe ID 1741 (ID 17725:
syscall::read:return): invalid address (0x53a000) in action #12 at DIF
offset 52
write(0xFFFFFFFF, "\0", 0x0)		 = -1 Err#9
dtrace: error on enabled probe ID 1741 (ID 17725:
syscall::read:return): invalid address (0x53a000) in action #12 at DIF
offset 52
write(0xFFFFFFFF, "\0", 0x0)		 = -1 Err#9
dtrace: error on enabled probe ID 1741 (ID 17725:
syscall::read:return): invalid address (0x53a000) in action #12 at DIF
offset 52
write(0xFFFFFFFF, "\0", 0x0)		 = -1 Err#9
dtrace: error on enabled probe ID 1741 (ID 17725:
syscall::read:return): invalid address (0x53a000) in action #12 at DIF
offset 52
write(0xFFFFFFFF, "\0", 0x0)		 = -1 Err#9
dtrace: error on enabled probe ID 1741 (ID 17725:
syscall::read:return): invalid address (0x53a000) in action #12 at DIF
offset 52
write(0xFFFFFFFF, "\0", 0x0)		 = -1 Err#9
dtrace: error on enabled probe ID 1741 (ID 17725:
syscall::read:return): invalid address (0x53a000) in action #12 at DIF
offset 52
write(0xFFFFFFFF, "\0", 0x0)		 = -1 Err#9
dtrace: error on enabled probe ID 1741 (ID 17725:
syscall::read:return): invalid address (0x53a000) in action #12 at DIF
offset 52
write(0xFFFFFFFF, "\0", 0x0)		 = -1 Err#9
dtrace: error on enabled probe ID 1741 (ID 17725:
syscall::read:return): invalid address (0x53a000) in action #12 at DIF
offset 52
write(0xFFFFFFFF, "\0", 0x0)		 = -1 Err#9
dtrace: error on enabled probe ID 1741 (ID 17725:
syscall::read:return): invalid address (0x53a000) in action #12 at DIF
offset 52
write(0xFFFFFFFF, "\0", 0x0)		 = -1 Err#9
dtrace: error on enabled probe ID 1741 (ID 17725:
syscall::read:return): invalid address (0x53a000) in action #12 at DIF
offset 52
write(0xFFFFFFFF, "\0", 0x0)		 = -1 Err#9
dtrace: error on enabled probe ID 1741 (ID 17725:
syscall::read:return): invalid address (0x53a000) in action #12 at DIF
offset 52
write(0xFFFFFFFF, "\0", 0x0)		 = -1 Err#9
dtrace: error on enabled probe ID 1741 (ID 17725:
syscall::read:return): invalid address (0x53a000) in action #12 at DIF
offset 52
write(0xFFFFFFFF, "\0", 0x0)		 = -1 Err#9
write(0xFFFFFFFF, "\0", 0x0)		 = -1 Err#9
dtrace: error on enabled probe ID 1741 (ID 17725:
syscall::read:return): invalid address (0x53a000) in action #12 at DIF
offset 52
write(0xFFFFFFFF, "\0", 0x0)		 = -1 Err#9
dtrace: error on enabled probe ID 1741 (ID 17725:
syscall::read:return): invalid address (0x53a000) in action #12 at DIF
offset 52
write(0xFFFFFFFF, "\0", 0x0)		 = -1 Err#9
dtrace: error on enabled probe ID 1741 (ID 17725:
syscall::read:return): invalid address (0x53a000) in action #12 at DIF
offset 52
write(0xFFFFFFFF, "\0", 0x0)		 = -1 Err#9
fcntl(0x4, 0x2, 0x1)		 = 0 0
fstat64(0x4, 0x524EE0, 0x1)		 = 0 0
read(0x4, "tmp/16gig.txt\0", 0x200)		 = 512 0
read(0x4, "\0", 0x400)		 = 1024 0
read(0x4, "\0", 0x800)		 = 2048 0
read(0x4, "\0", 0x1000)		 = 512 0
read(0x4, "\0", 0xE00)		 = 0 0
close(0x4)		 = 0 0
dtrace: error on enabled probe ID 1741 (ID 17725:
syscall::read:return): invalid address (0x53a000) in action #12 at DIF
offset 52
write(0xFFFFFFFF, "\0", 0x0)		 = -1 Err#9
dtrace: error on enabled probe ID 1741 (ID 17725:
syscall::read:return): invalid address (0x53a000) in action #12 at DIF
offset 52
write(0xFFFFFFFF, "\0", 0x0)		 = -1 Err#9
dtrace: error on enabled probe ID 1741 (ID 17725:
syscall::read:return): invalid address (0x53a000) in action #12 at DIF
offset 52
write(0xFFFFFFFF, "\0", 0x0)		 = -1 Err#9
dtrace: error on enabled probe ID 1741 (ID 17725:
syscall::read:return): invalid address (0x53a000) in action #12 at DIF
offset 52
write(0xFFFFFFFF, "\0", 0x0)		 = -1 Err#9
dtrace: error on enabled probe ID 1741 (ID 17725:
syscall::read:return): invalid address (0x53a000) in action #12 at DIF
offset 52
write(0xFFFFFFFF, "\0", 0x0)		 = -1 Err#9
dtrace: error on enabled probe ID 1741 (ID 17725:
syscall::read:return): invalid address (0x53a000) in action #12 at DIF
offset 52
write(0xFFFFFFFF, "\0", 0x0)		 = -1 Err#9
dtrace: error on enabled probe ID 1741 (ID 17725:
syscall::read:return): invalid address (0x53a000) in action #12 at DIF
offset 52
write(0xFFFFFFFF, "\0", 0x0)		 = -1 Err#9
dtrace: error on enabled probe ID 1741 (ID 17725:
syscall::read:return): invalid address (0x53a000) in action #12 at DIF
offset 52
write(0xFFFFFFFF, "\0", 0x0)		 = -1 Err#9
dtrace: error on enabled probe ID 1741 (ID 17725:
syscall::read:return): invalid address (0x53a000) in action #12 at DIF
offset 52
write(0xFFFFFFFF, "\0", 0x0)		 = -1 Err#9
dtrace: error on enabled probe ID 1741 (ID 17725:
syscall::read:return): invalid address (0x53a000) in action #12 at DIF
offset 52
write(0xFFFFFFFF, "\0", 0x0)		 = -1 Err#9
dtrace: error on enabled probe ID 1741 (ID 17725:
syscall::read:return): invalid address (0x53a000) in action #12 at DIF
offset 52
write(0xFFFFFFFF, "\0", 0x0)		 = -1 Err#9
dtrace: error on enabled probe ID 1741 (ID 17725:
syscall::read:return): invalid address (0x53a000) in action #12 at DIF
offset 52
write(0xFFFFFFFF, "\0", 0x0)		 = -1 Err#9
dtrace: error on enabled probe ID 1741 (ID 17725:
syscall::read:return): invalid address (0x53a000) in action #12 at DIF
offset 52
write(0xFFFFFFFF, "\0", 0x0)		 = -1 Err#9
dtrace: error on enabled probe ID 1741 (ID 17725:
syscall::read:return): invalid address (0x53a000) in action #12 at DIF
offset 52
write(0xFFFFFFFF, "\0", 0x0)		 = -1 Err#9
dtrace: error on enabled probe ID 1741 (ID 17725:
syscall::read:return): invalid address (0x53a000) in action #12 at DIF
offset 52
write(0xFFFFFFFF, "\0", 0x0)		 = -1 Err#9
dtrace: error on enabled probe ID 1741 (ID 17725:
syscall::read:return): invalid address (0x53a000) in action #12 at DIF
offset 52
write(0xFFFFFFFF, "\0", 0x0)		 = -1 Err#9
dtrace: error on enabled probe ID 1741 (ID 17725:
syscall::read:return): invalid address (0x53a000) in action #12 at DIF
offset 52
write(0xFFFFFFFF, "\0", 0x0)		 = -1 Err#9
dtrace: error on enabled probe ID 1741 (ID 17725:
syscall::read:return): invalid address (0x53a000) in action #12 at DIF
offset 52
write(0xFFFFFFFF, "\0", 0x0)		 = -1 Err#9
dtrace: error on enabled probe ID 1741 (ID 17725:
syscall::read:return): invalid address (0x53a000) in action #12 at DIF
offset 52
write(0xFFFFFFFF, "\0", 0x0)		 = -1 Err#9
dtrace: error on enabled probe ID 1741 (ID 17725:
syscall::read:return): invalid address (0x53a000) in action #12 at DIF
offset 52
write(0xFFFFFFFF, "\0", 0x0)		 = -1 Err#9
dtrace: error on enabled probe ID 1741 (ID 17725:
syscall::read:return): invalid address (0x53a000) in action #12 at DIF
offset 52
write(0xFFFFFFFF, "\0", 0x0)		 = -1 Err#9
dtrace: error on enabled probe ID 1741 (ID 17725:
syscall::read:return): invalid address (0x53a000) in action #12 at DIF
offset 52
write(0xFFFFFFFF, "\0", 0x0)		 = -1 Err#9
dtrace: error on enabled probe ID 1741 (ID 17725:
syscall::read:return): invalid address (0x53a000) in action #12 at DIF
offset 52
write(0xFFFFFFFF, "\0", 0x0)		 = -1 Err#9
dtrace: error on enabled probe ID 1741 (ID 17725:
syscall::read:return): invalid address (0x53a000) in action #12 at DIF
offset 52
write(0xFFFFFFFF, "\0", 0x0)		 = -1 Err#9
dtrace: error on enabled probe ID 1741 (ID 17725:
syscall::read:return): invalid address (0x53a000) in action #12 at DIF
offset 52
write(0xFFFFFFFF, "\0", 0x0)		 = -1 Err#9
dtrace: error on enabled probe ID 1741 (ID 17725:
syscall::read:return): invalid address (0x53a000) in action #12 at DIF
offset 52
write(0xFFFFFFFF, "\0", 0x0)		 = -1 Err#9
dtrace: error on enabled probe ID 1741 (ID 17725:
syscall::read:return): invalid address (0x53a000) in action #12 at DIF
offset 52
write(0xFFFFFFFF, "\0", 0x0)		 = -1 Err#9
dtrace: error on enabled probe ID 1741 (ID 17725:
syscall::read:return): invalid address (0x53a000) in action #12 at DIF
offset 52
write(0xFFFFFFFF, "\0", 0x0)		 = -1 Err#9
dtrace: error on enabled probe ID 1741 (ID 17725:
syscall::read:return): invalid address (0x53a000) in action #12 at DIF
offset 52
write(0xFFFFFFFF, "\0", 0x0)		 = -1 Err#9
dtrace: error on enabled probe ID 1741 (ID 17725:
syscall::read:return): invalid address (0x53a000) in action #12 at DIF
offset 52
write(0xFFFFFFFF, "\0", 0x0)		 = -1 Err#9
dtrace: error on enabled probe ID 1741 (ID 17725:
syscall::read:return): invalid address (0x53a000) in action #12 at DIF
offset 52
write(0xFFFFFFFF, "\0", 0x0)		 = -1 Err#9
dtrace: error on enabled probe ID 1741 (ID 17725:
syscall::read:return): invalid address (0x53a000) in action #12 at DIF
offset 52
write(0xFFFFFFFF, "\0", 0x0)		 = -1 Err#9

[cw, 2009-12-11 23:33:54]

On Fri, Dec 11, 2009 at 03:07:13PM -0800, Russ Cox wrote:

> Only fails on darwin/386.  darwin/amd64 and linux/* seem to work.

I wonder if pipes are working on that platform.  Let me try to get a
test env for that and look into it.

[cw, 2009-12-12 06:21:09]

The problem is darwin 386 syscall interface for seek doesn't return errors.  I
manually patched this in by hand to test it and it does indeed work correctly.

I need to find out how to fix mksyscall.sh to do this properly.

[cw, 2009-12-12 06:34:22]

On Sat, Dec 12, 2009 at 06:21:09AM +0000, cw@f00f.org wrote:

> I need to find out how to fix mksyscall.sh to do this properly.

OK, I have a fix for that.  I'll submit that shortly then resubmit
this one.

[cw, 2009-12-12 06:49:40]

Hello rsc, dsymonds1,

I'd like you to review the following change.

[rsc, 2009-12-14 20:23:52]

*** Submitted as http://code.google.com/p/go/source/detail?r=a33ae1785d42 ***

archive/tar: bug fixes.
1. If all data is exhausted using Read then a following Next will
   fail as if it saw EOF.  (Test case added.)
2. Seeking isn't always possible (i.e. sockets and pipes).  Fallback
   to read.  (Test case added.)
3. Fix to readHeader (cleaner fix pointed out by rsc).
   (TestReader modified.)
4. When Read has consumed all the data, don't try to read 0 bytes from reader.
   In cases where tr.nb is zero we attempt to read zero bytes and thus
   never see an EOF (this is most easily seen when the 'tar source' is
   something like bytes.Buffer{} as opposed to os.File).
5. If write is used to the point of ErrWriteTooLong, allow additional file
entries.
6. Make close work as expected.  That is any further Write or
   WriteHeader attempts will result in ErrWriteAfterClose.
Fixes  issue 419 .

R=rsc, dsymonds1
http://codereview.appspot.com/162062

Committer: Russ Cox <rsc@golang.org>