code review 4807042: godoc: fix zip file directory lookup

godoc: fix zip file directory lookup

Also: remove left-over println calls.

[gri, 2011-07-20 23:10:38]

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

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

[bradfitz, 2011-07-20 23:15:56]

LGTM

On Wed, Jul 20, 2011 at 4:10 PM, <gri@golang.org> wrote:

> Reviewers: bradfitz,
>
> Message:
> Hello bradfitz@golang.org (cc: golang-dev@googlegroups.com),
>
> I'd like you to review this change to
> https://go.googlecode.com/hg/
>
>
> Description:
> godoc: fix zip file directory lookup
>
> Also: remove left-over println calls.
>
> Please review this at
http://codereview.appspot.com/**4807042/<http://codereview.appspot.com/4807042/>
>
> Affected files:
>  M src/cmd/godoc/httpzip.go
>  M src/cmd/godoc/zip.go
>
>
> Index: src/cmd/godoc/httpzip.go
> ==============================**==============================**=======
> --- a/src/cmd/godoc/httpzip.go
> +++ b/src/cmd/godoc/httpzip.go
> @@ -61,7 +61,6 @@
>  }
>
>  func (f *httpZipFile) Readdir(count int) ([]os.FileInfo, os.Error) {
> -       println("Readdir", f.info.Name)
>        if f.info.IsRegular() {
>                return nil, fmt.Errorf("Readdir called for regular file:
> %s", f.info.Name)
>        }
> @@ -134,14 +133,15 @@
>  }
>
>  func (fs *httpZipFS) Open(abspath string) (http.File, os.Error) {
> -       name := path.Join(fs.root, abspath)
> -       index := fs.list.lookup(name)
> +       name := path.Join(fs.root, abspath) // name is clean
> +       index, exact := fs.list.lookup(name)
>        if index < 0 {
>                return nil, fmt.Errorf("file not found: %s", abspath)
>        }
>
> -       if f := fs.list[index]; f.Name == name {
> +       if exact {
>                // exact match found - must be a file
> +               f := fs.list[index]
>                rc, err := f.Open()
>                if err != nil {
>                        return nil, err
> @@ -159,7 +159,6 @@
>        }
>
>        // not an exact match - must be a directory
> -       println("opened directory", abspath, len(fs.list[index:]))
>        return &httpZipFile{
>                os.FileInfo{
>                        Name: abspath,
> Index: src/cmd/godoc/zip.go
> ==============================**==============================**=======
> --- a/src/cmd/godoc/zip.go
> +++ b/src/cmd/godoc/zip.go
> @@ -73,22 +73,23 @@
>  }
>
>  func zipPath(name string) string {
> +       name = path.Clean(name)
>        if !path.IsAbs(name) {
>                panic(fmt.Sprintf("stat: not an absolute path: %s", name))
>        }
> -       return name[1:] // strip '/'
> +       return name[1:] // strip leading '/'
>  }
>
>  func (fs *zipFS) stat(abspath string) (int, zipFI, os.Error) {
> -       i := fs.list.lookup(abspath)
> +       i, exact := fs.list.lookup(abspath)
>        if i < 0 {
>                return -1, zipFI{}, fmt.Errorf("file not found: %s",
> abspath)
>        }
> +       _, name := path.Split(abspath)
>        var file *zip.File
> -       if abspath == fs.list[i].Name {
> +       if exact {
>                file = fs.list[i] // exact match found - must be a file
>        }
> -       _, name := path.Split(abspath)
>        return i, zipFI{name, file}, nil
>  }
>
> @@ -173,17 +174,29 @@
>  func (z zipList) Less(i, j int) bool { return z[i].Name < z[j].Name }
>  func (z zipList) Swap(i, j int)      { z[i], z[j] = z[j], z[i] }
>
> -// lookup returns the first index in the zipList
> -// of a path equal to name or beginning with name/.
> -func (z zipList) lookup(name string) int {
> +// lookup returns the smallest index of an entry with an exact match
> +// for name, or an inexact match starting with name/. If there is no
> +// such entry, the result is -1, false.
> +func (z zipList) lookup(name string) (index int, exact bool) {
> +       // look for exact match first (name comes before name/ in z)
>        i := sort.Search(len(z), func(i int) bool {
>                return name <= z[i].Name
>        })
> -       if i >= 0 {
> -               iname := z[i].Name
> -               if strings.HasPrefix(iname, name) && (len(name) ==
> len(iname) || iname[len(name)] == '/') {
> -                       return i
> -               }
> +       if i < 0 {
> +               return -1, false
>        }
> -       return -1 // no match
> +       if z[i].Name == name {
> +               return i, true
> +       }
> +
> +       // look for inexact match (must be in z[i:], if present)
> +       z = z[i:]
> +       name += "/"
> +       j := sort.Search(len(z), func(i int) bool {
> +               return name <= z[i].Name
> +       })
> +       if j < 0 {
> +               return -1, false
> +       }
> +       return i + j, false
>  }
>
>
>

[gri, 2011-07-20 23:22:15]

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

godoc: fix zip file directory lookup

Also: remove left-over println calls.

R=bradfitz
CC=golang-dev
http://codereview.appspot.com/4807042