code review 5754043: expvar: add locking to String, and use RWMutex properly...

expvar: add locking to String, and use RWMutex properly throughout.

[dsymonds, 2012-03-05 22:06:03]

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

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

[bradfitz, 2012-03-05 22:09:57]

LGTM

whoops, not sure how I missed *String.  probably skipped over it too
quickly when I was searching for "String", thinking I was looking at the
type, not the method.


On Mon, Mar 5, 2012 at 2:06 PM, <dsymonds@golang.org> wrote:

> Reviewers: bradfitz,
>
> Message:
> Hello bradfitz (cc: golang-dev@googlegroups.com),
>
> I'd like you to review this change to
> https://go.googlecode.com/hg/
>
>
> Description:
> expvar: add locking to String, and use RWMutex properly throughout.
>
> Please review this at
http://codereview.appspot.com/**5754043/<http://codereview.appspot.com/5754043/>
>
> Affected files:
>  M src/pkg/expvar/expvar.go
>
>
> Index: src/pkg/expvar/expvar.go
> ==============================**==============================**=======
> --- a/src/pkg/expvar/expvar.go
> +++ b/src/pkg/expvar/expvar.go
> @@ -41,12 +41,12 @@
>  // Int is a 64-bit integer variable that satisfies the Var interface.
>  type Int struct {
>        i  int64
> -       mu sync.Mutex
> +       mu sync.RWMutex
>  }
>
>  func (v *Int) String() string {
> -       v.mu.Lock()
> -       defer v.mu.Unlock()
> +       v.mu.RLock()
> +       defer v.mu.RUnlock()
>        return strconv.FormatInt(v.i, 10)
>  }
>
> @@ -65,12 +65,12 @@
>  // Float is a 64-bit float variable that satisfies the Var interface.
>  type Float struct {
>        f  float64
> -       mu sync.Mutex
> +       mu sync.RWMutex
>  }
>
>  func (v *Float) String() string {
> -       v.mu.Lock()
> -       defer v.mu.Unlock()
> +       v.mu.RLock()
> +       defer v.mu.RUnlock()
>        return strconv.FormatFloat(v.f, 'g', -1, 64)
>  }
>
> @@ -188,12 +188,21 @@
>
>  // String is a string variable, and satisfies the Var interface.
>  type String struct {
> -       s string
> +       s  string
> +       mu sync.RWMutex
>  }
>
> -func (v *String) String() string { return strconv.Quote(v.s) }
> +func (v *String) String() string {
> +       v.mu.RLock()
> +       defer v.mu.RUnlock()
> +       return strconv.Quote(v.s)
> +}
>
> -func (v *String) Set(value string) { v.s = value }
> +func (v *String) Set(value string) {
> +       v.mu.Lock()
> +       defer v.mu.Unlock()
> +       v.s = value
> +}
>
>  // Func implements Var by calling the function
>  // and formatting the returned value using JSON.
>
>
>

[dsymonds, 2012-03-05 22:13:36]

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

expvar: add locking to String, and use RWMutex properly throughout.

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