code review 4044043: gob: report an error when encoding a non-empty struct with

gob: report an error when encoding a non-empty struct with
no public fields.
Fix a couple of tests caught out by this change.

[r, 2011-01-22 04:36:43]

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

I'd like you to review this change.

[rsc, 2011-01-22 06:27:14]

LGTM

On Friday, January 21, 2011,  <r@golang.org> wrote:
> Reviewers: rsc,
>
> Message:
> Hello rsc (cc: golang-dev@googlegroups.com),
>
> I'd like you to review this change.
>
>
> Description:
> gob: report an error when encoding a non-empty struct with
> no public fields.
> Fix a couple of tests caught out by this change.
>
> Please review this at http://codereview.appspot.com/4044043/
>
> Affected files:
>   M src/pkg/gob/codec_test.go
>   M src/pkg/gob/encode.go
>   M src/pkg/gob/encoder_test.go
>
>
> Index: src/pkg/gob/codec_test.go
> ===================================================================
> --- a/src/pkg/gob/codec_test.go
> +++ b/src/pkg/gob/codec_test.go
> @@ -1088,11 +1088,11 @@
>  }
>
>  type Point struct {
> -       a, b int
> +       X, Y int
>  }
>
>  func (p Point) Square() int {
> -       return p.a*p.a + p.b*p.b
> +       return p.X*p.X + p.Y*p.Y
>  }
>
>  // A struct with interfaces in it.
> Index: src/pkg/gob/encode.go
> ===================================================================
> --- a/src/pkg/gob/encode.go
> +++ b/src/pkg/gob/encode.go
> @@ -264,9 +264,6 @@
>         }
>  }
>
> -func encNoOp(i *encInstr, state *encoderState, p unsafe.Pointer) {
> -}
> -
>  // Byte arrays are encoded as an unsigned count followed by the raw bytes.
>  func encUint8Array(i *encInstr, state *encoderState, p unsafe.Pointer) {
>         b := *(*[]byte)(p)
> @@ -516,16 +513,18 @@
>         srt, isStruct := rt.(*reflect.StructType)
>         engine := new(encEngine)
>         if isStruct {
> -               engine.instr = make([]encInstr, srt.NumField()+1) // +1 for
terminator
> -               for fieldnum := 0; fieldnum < srt.NumField(); fieldnum++ {
> -                       f := srt.Field(fieldnum)
> +               for fieldNum := 0; fieldNum < srt.NumField(); fieldNum++ {
> +                       f := srt.Field(fieldNum)
> +                       if !isExported(f.Name) {
> +                               continue
> +                       }
>                         op, indir := enc.encOpFor(f.Type)
> -                       if !isExported(f.Name) {
> -                               op = encNoOp
> -                       }
> -                       engine.instr[fieldnum] = encInstr{op, fieldnum, indir,
uintptr(f.Offset)}
> +                       engine.instr = append(engine.instr, encInstr{op,
fieldNum, indir, uintptr(f.Offset)})
>                 }
> -               engine.instr[srt.NumField()] = encInstr{encStructTerminator,
0, 0, 0}
> +               if srt.NumField() > 0 && len(engine.instr) == 0 {
> +                       errorf("type %s has no exported fields", rt)
> +               }
> +               engine.instr = append(engine.instr,
encInstr{encStructTerminator, 0, 0, 0})
>         } else {
>                 engine.instr = make([]encInstr, 1)
>                 op, indir := enc.encOpFor(rt)
> Index: src/pkg/gob/encoder_test.go
> ===================================================================
> --- a/src/pkg/gob/encoder_test.go
> +++ b/src/pkg/gob/encoder_test.go
> @@ -220,7 +220,7 @@
>  func TestValueError(t *testing.T) {
>         // Encode a *T, decode a T
>         type Type4 struct {
> -               a int
> +               A int
>         }
>         t4p := &Type4{3}
>         var t4 Type4 // note: not a pointer.
>
>
>

[r, 2011-01-22 08:10:17]

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

gob: report an error when encoding a non-empty struct with
no public fields.
Fix a couple of tests caught out by this change.

R=rsc
CC=golang-dev
http://codereview.appspot.com/4044043