code review 15020050: leveldb: use s[i:j:k] syntax.

leveldb: use s[i:j:k] syntax.

[nigeltao, 2013-10-22 06:37:18]

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

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

[dsymonds, 2013-10-22 06:40:15]

You're ready to ditch Go 1.1 compatibility already?

[adg, 2013-10-22 07:03:25]

Why not? It's not like leveldb-go is usable yet.



On 22 October 2013 10:40, David Symonds <dsymonds@golang.org> wrote:

> You're ready to ditch Go 1.1 compatibility already?
>
> --
>
> ---
> You received this message because you are subscribed to the Google Groups
> "golang-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to golang-dev+unsubscribe@googlegroups.com.
> For more options, visit https://groups.google.com/groups/opt_out.
>

[dsymonds, 2013-10-22 07:44:46]

I've heard of people using it already.

[bradfitz, 2013-10-22 12:45:07]

More than just me using a small part of it? I have a copy in my third_party
anyway.

I think assuming 1.2 is fine.
 On Oct 22, 2013 12:44 AM, "David Symonds" <dsymonds@golang.org> wrote:

> I've heard of people using it already.
>

[bradfitz, 2013-10-22 15:55:16]

LGTM



On Mon, Oct 21, 2013 at 11:37 PM, <nigeltao@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://code.google.com/p/**leveldb-go<https://code.google.com/p/leveldb-go>
>
>
> Description:
> leveldb: use s[i:j:k] syntax.
>
> Please review this at
https://codereview.appspot.**com/15020050/<https://codereview.appspot.com/150...
>
> Affected files (+21, -3 lines):
>   M leveldb/memdb/memdb.go
>   M leveldb/memdb/memdb_test.go
>   M leveldb/table/reader.go
>   M leveldb/table/table_test.go
>
>
> Index: leveldb/memdb/memdb.go
> ==============================**==============================**=======
> --- a/leveldb/memdb/memdb.go
> +++ b/leveldb/memdb/memdb.go
> @@ -86,7 +86,8 @@
>                 return nil
>         }
>         bLen, n := binary.Uvarint(m.kvData[**kvOffset:])
> -       return m.kvData[kvOffset+n : kvOffset+n+int(bLen)]
> +       i, j := kvOffset+n, kvOffset+n+int(bLen)
> +       return m.kvData[i:j:j]
>  }
>
>  // save saves a []byte to m.kvData.
> Index: leveldb/memdb/memdb_test.go
> ==============================**==============================**=======
> --- a/leveldb/memdb/memdb_test.go
> +++ b/leveldb/memdb/memdb_test.go
> @@ -167,6 +167,9 @@
>                 j := r.Intn(N)
>                 k := []byte(strconv.Itoa(j))
>                 v, err := m0.Get(k, nil)
> +               if len(v) != cap(v) {
> +                       t.Fatalf("get: j=%d, got len(v)=%d, cap(v)=%d", j,
> len(v), cap(v))
> +               }
>                 var c uint8
>                 if len(v) != 0 {
>                         c = v[0]
> @@ -210,6 +213,12 @@
>                 if got := string(x.Key()); got != want {
>                         t.Fatalf("iter: got %q, want %q", got, want)
>                 }
> +               if k := x.Key(); len(k) != cap(k) {
> +                       t.Fatalf("iter: len(k)=%d, cap(k)=%d", len(k),
> cap(k))
> +               }
> +               if v := x.Value(); len(v) != cap(v) {
> +                       t.Fatalf("iter: len(v)=%d, cap(v)=%d", len(v),
> cap(v))
> +               }
>         }
>         if err := x.Close(); err != nil {
>                 t.Fatalf("close: %v", err)
> Index: leveldb/table/reader.go
> ==============================**==============================**=======
> --- a/leveldb/table/reader.go
> +++ b/leveldb/table/reader.go
> @@ -132,7 +132,7 @@
>         if i.soi {
>                 return nil
>         }
> -       return i.key
> +       return i.key[:len(i.key):len(i.key)]
>  }
>
>  // Value implements Iterator.Value, as documented in the leveldb/db
> package.
> @@ -140,7 +140,7 @@
>         if i.soi {
>                 return nil
>         }
> -       return i.val
> +       return i.val[:len(i.val):len(i.val)]
>  }
>
>  // Close implements Iterator.Close, as documented in the leveldb/db
> package.
> Index: leveldb/table/table_test.go
> ==============================**==============================**=======
> --- a/leveldb/table/table_test.go
> +++ b/leveldb/table/table_test.go
> @@ -54,6 +54,8 @@
>                 // Check using Get.
>                 if v1, err := r.Get([]byte(k), nil); string(v1) !=
> string(v) || err != nil {
>                         return fmt.Errorf("Get %q: got (%q, %v), want (%q,
> %v)", k, v1, err, v, error(nil))
> +               } else if len(v1) != cap(v1) {
> +                       return fmt.Errorf("Get %q: len(v1)=%d,
> cap(v1)=%d", k, len(v1), cap(v1))
>                 }
>
>                 // Check using Find.
> @@ -61,9 +63,15 @@
>                 if !i.Next() || string(i.Key()) != k {
>                         return fmt.Errorf("Find %q: key was not in the
> table", k)
>                 }
> +               if k1 := i.Key(); len(k1) != cap(k1) {
> +                       return fmt.Errorf("Find %q: len(k1)=%d,
> cap(k1)=%d", k, len(k1), cap(k1))
> +               }
>                 if string(i.Value()) != v {
>                         return fmt.Errorf("Find %q: got value %q, want
> %q", k, i.Value(), v)
>                 }
> +               if v1 := i.Value(); len(v1) != cap(v1) {
> +                       return fmt.Errorf("Find %q: len(v1)=%d,
> cap(v1)=%d", k, len(v1), cap(v1))
> +               }
>                 if err := i.Close(); err != nil {
>                         return err
>                 }
>
>
>

[nigeltao, 2013-10-22 21:09:50]

On Tue, Oct 22, 2013 at 5:40 PM, David Symonds <dsymonds@golang.org> wrote:
> You're ready to ditch Go 1.1 compatibility already?

Yes.

[nigeltao, 2013-10-22 22:29:41]

*** Submitted as
https://code.google.com/p/leveldb-go/source/detail?r=90cce1f0216b ***

leveldb: use s[i:j:k] syntax.

R=bradfitz, dsymonds, adg
CC=golang-dev
https://codereview.appspot.com/15020050