code review 7203050: archive/tar: provide FileInfo method on Header

archive/tar: provide FileInfo method on Header

No tests yet, just wanted to confirm whether TestRoundTrip in tar_test.go was the best location?
Also wanted to check whether the Mode() logic is correct in common.go?
Final question, is it worth creating a tar file to test restoring block devices and sockets? I've never done this and don't know if it is a common usage of tar.

Fixes issue 4695.

[Donovan, 2013-01-24 12:05:13]

Hello golang-dev@googlegroups.com (cc: adg@golang.org),

I'd like you to review this change to
https://code.google.com/p/go

[adg, 2013-01-24 12:11:21]

On 24 January 2013 23:05, <DonovanHide@gmail.com> wrote:

> Reviewers: golang-dev_googlegroups.com,
>
> Message:
> Hello golang-dev@googlegroups.com (cc: adg@golang.org),
>
> I'd like you to review this change to
> https://code.google.com/p/go
>
>
> Description:
> archive/tar: provide FileInfo method on Header
>
> Hi, this is my first attempt at submitting a CL, so please excuse any
> obvious mistakes!
>

Thanks! Now that you've introduced yourself, you should "hg change 7203050"
and remove this message from your change description. (It's what ends up in
our source control history.)


> No tests yet, just wanted to confirm whether TestRoundTrip in
> tar_test.go was the best location?


Sounds good.


>         Also wanted to check whether
> the Mode() logic is correct in common.go?
>

Will respond with review comments (shortly).


> Final question, is it worth creating a tar file to test restoring block
> devices and sockets? I've never done this and don't know if it is a
> common usage of tar.
>

I don't know either. You could give it a try; if it's easy, then do it.
Just make sure you check runtime.GOOS in the test and use t.Skip under
windows and plan9.

Andrew

[adg, 2013-01-24 12:13:20]

https://codereview.appspot.com/7203050/diff/3/src/pkg/archive/tar/common.go
File src/pkg/archive/tar/common.go (right):

https://codereview.appspot.com/7203050/diff/3/src/pkg/archive/tar/common.go#n...
src/pkg/archive/tar/common.go:85: func (fi headerFileInfo) Mode() (mode
os.FileMode) {
put a blank line above this one; keep the 1-liners together, and let the larger
function stand alone

https://codereview.appspot.com/7203050/diff/3/src/pkg/archive/tar/common.go#n...
src/pkg/archive/tar/common.go:88: switch {
Are these flags supposed to be exclusive? Or should it be a series of if
statements? I think the code is probably correct, just checking.

https://codereview.appspot.com/7203050/diff/3/src/pkg/archive/tar/common.go#n...
src/pkg/archive/tar/common.go:112: func (fi headerFileInfo) Sys() interface{} {
return fi.fh }
put this up with the other 1-liners

[remyoudompheng, 2013-06-05 03:51:23]

R=close

This patch is obsoleted by revision de92672228d3
http://code.google.com/p/go/source/detail?r=de92672228d3