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

encoding/gob: Int, float arrays don't encode/decode correctly #459

Closed
gopherbot opened this issue Dec 24, 2009 · 10 comments
Closed

encoding/gob: Int, float arrays don't encode/decode correctly #459

gopherbot opened this issue Dec 24, 2009 · 10 comments
Assignees

Comments

@gopherbot
Copy link

@gopherbot gopherbot commented Dec 24, 2009

by soniakeys:

Before filing a bug, please check whether it has been fixed since
the latest release: run "hg pull -u" and retry what you did to
reproduce the problem.  Thanks.

What steps will reproduce the problem?
1.  modify TestArray (http://pastebin.com/f1301c4ed) of encoder_test.go to
test an int array instead of a string array.
2.  modifed code:  http://pastebin.com/f208c03e9
3.  gotest

What is the expected output? What do you see instead?
PASS expected.  EOF and FAIL seen.

What is your $GOOS?  $GOARCH?
linux, amd64.


Which revision are you using?  (hg identify)
release.2009-12-09

Please provide any additional information below.
Code I was trying with float arrays either crashed or decoded wrong
numbers.  I expanded TestArray a little more to actually verify values. 
http://pastebin.com/f294d2205  Output from this was 3 "data corruption"
messages.
@krasin
Copy link

@krasin krasin commented Dec 24, 2009

Comment 1:

I have started to work on this issue. It fails on zeros in arrays. Also, there is a 
separate issue with arrays (they could not be encoded directly; only in structs)
@krasin
Copy link

@krasin krasin commented Dec 24, 2009

Comment 2:

The second issue is known issue and I will not resolve it now. I'm working on fixing 
first.
@krasin
Copy link

@krasin krasin commented Dec 24, 2009

Comment 3:

The problem is in the encoder. It encodes int array 1 0 1 and 1 1 0 equally.
Here is a chunk for the array 6 78 1 3 2 2 0
6 - number of bytes in the rest of chunk
78 - type
1 - type
3 - array length
2 - 1
2 - 1
0 - end of array
Zeros are just not encoded. Here is 1 0 1 0 1 0 1 chunk:
8 78 1 7 2 2 2 2 0
Actually, decoder just can't guess where zeros are.
@krasin
Copy link

@krasin krasin commented Dec 24, 2009

Comment 4:

Ok, the problem is obvious but I don't know how to fix it. encode.go states:
func encInt32(i *encInstr, state *encoderState, p unsafe.Pointer) {
    v := int64(*(*int32)(p))
    if v != 0 {
        state.update(i)
        encodeInt(state, v)
    }
}
Similar condition (v != 0) exists for all types. So, we just miss array elements! 
Since this condition was introduced with some (unknown for me) intention, I'd like to  
hear rationale for this from authors of this code.
@krasin
Copy link

@krasin krasin commented Dec 24, 2009

Comment 5:

My current understanding it was a premature optimization to avoid storing default 
values of struct fields. It's probably a good idea for some cases, but the 
implementation leads to a number of failed tests with arrays.
I have create a quick fix: http://golang.org/cl/183060
@robpike
Copy link
Contributor

@robpike robpike commented Dec 28, 2009

Comment 6:

The quick fix is too brutal.

Owner changed to r...@golang.org.

Status changed to Accepted.

@krasin
Copy link

@krasin krasin commented Dec 28, 2009

Comment 7:

Suggestions?
@gopherbot
Copy link
Author

@gopherbot gopherbot commented Dec 28, 2009

Comment 8 by soniakeys:

I'm thinking run-length encoding in the value encoding would be nice.  maybe a
positive number could mean that many different values would follow and a negative
number could mean repeat the following value -n times.  0 0 0 0 87 3 2 2 2 could be
encoded as -4 0 2 87 3 -3 2.  I'm no where near ready to propose code for this
though.  I'm still studying the existing code, trying to figure out how it all works.
@robpike
Copy link
Contributor

@robpike robpike commented Dec 28, 2009

Comment 9:

I have a fix out for review http://golang.org/cl/181073
It's just a trivial bug: defaults need to be sent for array elements.  The encoding
could be cleverer but I don't think it's necessary.
The code is not easy to follow.  I might rewrite it once the design is shaken down
some more.
@robpike
Copy link
Contributor

@robpike robpike commented Dec 28, 2009

Comment 10:

This issue was closed by revision 33311a7.

Status changed to Fixed.

Merged into issue #-.

@gopherbot gopherbot added the fixed label Dec 28, 2009
@mikioh mikioh changed the title Int, float arrays don't encode/decode correctly in gob package encoding/gob: Int, float arrays don't encode/decode correctly Feb 26, 2015
@golang golang locked and limited conversation to collaborators Jun 24, 2016
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.