code review 4431075: xml: fix reflect error

xml: fix reflect error

Fixes issue 1749.

[rsc, 2011-04-28 19:04:06]

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

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

[bradfitz, 2011-04-28 19:41:54]

LGTM

On Thu, Apr 28, 2011 at 12:04 PM, <rsc@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:
> xml: fix reflect error
>
> Fixes issue 1749.
>
> Please review this at http://codereview.appspot.com/4431075/
>
> Affected files:
>  M src/pkg/xml/read.go
>  M src/pkg/xml/xml_test.go
>
>
> Index: src/pkg/xml/read.go
> ===================================================================
> --- a/src/pkg/xml/read.go
> +++ b/src/pkg/xml/read.go
> @@ -220,13 +220,10 @@
>        }
>
>        if pv := val; pv.Kind() == reflect.Ptr {
> -               if pv.Pointer() == 0 {
> -                       zv := reflect.Zero(pv.Type().Elem())
> -                       pv.Set(zv.Addr())
> -                       val = zv
> -               } else {
> -                       val = pv.Elem()
> +               if pv.IsNil() {
> +                       pv.Set(reflect.New(pv.Type().Elem()))
>                }
> +               val = pv.Elem()
>        }
>
>        var (
> Index: src/pkg/xml/xml_test.go
> ===================================================================
> --- a/src/pkg/xml/xml_test.go
> +++ b/src/pkg/xml/xml_test.go
> @@ -347,6 +347,7 @@
>        Float32 float32
>        Float64 float64
>        String  string
> +       PtrString       *string
>  }
>
>  var all = allScalars{
> @@ -368,8 +369,11 @@
>        Float32: 13.0,
>        Float64: 14.0,
>        String:  "15",
> +       PtrString: &sixteen,
>  }
>
> +var sixteen = "16"
> +
>  const testScalarsInput = `<allscalars>
>        <true1>true</true1>
>        <true2>1</true2>
> @@ -390,6 +394,7 @@
>        <float32>13.0</float32>
>        <float64>14.0</float64>
>        <string>15</string>
> +       <ptrstring>16</ptrstring>
>  </allscalars>`
>
>  func TestAllScalars(t *testing.T) {
> @@ -401,7 +406,7 @@
>                t.Fatal(err)
>        }
>        if !reflect.DeepEqual(a, all) {
> -               t.Errorf("expected %+v got %+v", all, a)
> +               t.Errorf("have %+v want %+v", a, all)
>        }
>  }
>
>
>
>

[rsc, 2011-04-28 19:43:49]

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

xml: fix reflect error

Fixes issue 1749.

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