Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Slice struct with all "omitempty" fields set to zero value ​​will generate wrong data #376

Closed
ljfuyuan opened this issue Oct 29, 2024 · 1 comment · Fixed by #377
Closed

Comments

@ljfuyuan
Copy link

look at this sample below:

type Sample struct {
	K        uint32 `msg:"k,omitempty"`
        V        uint32 `msg:"v,omitempty"`
}

type Samples []Sample

and now we construct a set of test cases:

s := Samples{
		{1, 1},
		{0, 0},
		{3, 3},
	}

and the generated code is:

	for za0001 := range z {
		// check for omitted fields
		zb0001Len := uint32(2)
		var zb0001Mask uint8 /* 2 bits */
		_ = zb0001Mask
		if z[za0001].K == 0 {
			zb0001Len--
			zb0001Mask |= 0x1
		}
		if z[za0001].V == 0 {
			zb0001Len--
			zb0001Mask |= 0x2
		}
		// variable map header, size zb0001Len
		o = append(o, 0x80|uint8(zb0001Len))
		if zb0001Len == 0 {
			return     //  <---- !!! When looping to the second element, wrong data will be generated due to return. Continue may be used here. 

		}
...... ignore
klauspost added a commit to klauspost/msgp that referenced this issue Oct 29, 2024
Instead of doing return, insert an if block to skip emitting zero fields.

This also allows the check to be inserted at any level.

Fixes tinylib#376

Emitted code:

```
// MarshalMsg implements msgp.Marshaler
func (z TypeSample) MarshalMsg(b []byte) (o []byte, err error) {
	o = msgp.Require(b, z.Msgsize())
	// check for omitted fields
	zb0001Len := uint32(2)
	var zb0001Mask uint8 /* 2 bits */
	_ = zb0001Mask
	if z.K == 0 {
		zb0001Len--
		zb0001Mask |= 0x1
	}
	if z.V == 0 {
		zb0001Len--
		zb0001Mask |= 0x2
	}
	// variable map header, size zb0001Len
	o = append(o, 0x80|uint8(zb0001Len))
	// Skip if no fields are to be emitted
	if zb0001Len != 0 {
		if (zb0001Mask & 0x1) == 0 { // if not omitted
			// string "k"
			o = append(o, 0xa1, 0x6b)
			o = msgp.AppendUint32(o, z.K)
		}
		if (zb0001Mask & 0x2) == 0 { // if not omitted
			// string "v"
			o = append(o, 0xa1, 0x76)
			o = msgp.AppendUint32(o, z.V)
		}
	}
	return
}
```

If only 1 field, the check is omitted (and there is similar behavior on clearomitted).
@klauspost
Copy link
Collaborator

Please try #377

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants