code review 5124044: archive/zip: add support for reading and writing unix f...

archive/zip: read and write unix file modes

[niemeyer, 2011-09-25 01:40:53]

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

I'd like you to review this change to
https://go.googlecode.com/hg/

[rsc, 2011-09-25 03:05:52]

Please revert the os/types.go changes.
'reports whether' is the intended wording for bools.

Russ

[adg, 2011-09-25 04:27:48]

http://codereview.appspot.com/5124044/diff/5007/src/pkg/archive/zip/struct.go
File src/pkg/archive/zip/struct.go (right):

http://codereview.appspot.com/5124044/diff/5007/src/pkg/archive/zip/struct.go...
src/pkg/archive/zip/struct.go:34: creatorUnix = 3
Where did this come from? Is it setting two bits, or is there something else?
Should this just be 3<<8 here?

[niemeyer, 2011-09-25 17:15:53]

PTAL


[Russ]
> Please revert the os/types.go changes.
> 'reports whether' is the intended wording for bools.

I thought I had seen the opposite being agreed to, but
clearly I got it wrong. Reverted.


[Andrew]

http://codereview.appspot.com/5124044/diff/5007/src/pkg/archive/zip/struct.go
File src/pkg/archive/zip/struct.go (right):

http://codereview.appspot.com/5124044/diff/5007/src/pkg/archive/zip/struct.go...
src/pkg/archive/zip/struct.go:34: creatorUnix = 3
> Where did this come from? Is it setting two bits,
> or is there something else?

Comes from the documentation for the relevant field in
the documentation pointed at in line 8 of this file,
section J:

"""
The upper byte indicates the compatibility of the file
attribute information.  If the external file attributes 
(...)
"""

> Should this just be 3<<8 here?

Works either way for me. There's just no practical benefit
in shifting the constant in the documentation at the moment.

[adg, 2011-09-25 23:04:45]

LGTM

http://codereview.appspot.com/5124044/diff/1003/src/pkg/archive/zip/struct.go
File src/pkg/archive/zip/struct.go (right):

http://codereview.appspot.com/5124044/diff/1003/src/pkg/archive/zip/struct.go...
src/pkg/archive/zip/struct.go:34: creatorUnix = 3
Please put this up in the previous const block and add a doc comment.

[niemeyer, 2011-09-25 23:48:15]

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

archive/zip: read and write unix file modes

R=golang-dev, rsc, adg
CC=golang-dev
http://codereview.appspot.com/5124044