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

Marshall functions should consider length instead of writing whole set #114

Closed
omerfirmak opened this issue Nov 16, 2022 · 1 comment · Fixed by #115
Closed

Marshall functions should consider length instead of writing whole set #114

omerfirmak opened this issue Nov 16, 2022 · 1 comment · Fixed by #115

Comments

@omerfirmak
Copy link
Contributor

Due to calls like Delete(), size of set and length could be out of sync.

Whole b.set is marshaled;

	for i := range b.set {
		binaryOrder.PutUint64(item, b.set[i])
		if nn, err := writer.Write(item); err != nil {
			return int64(i*binary.Size(uint64(0)) + nn), err
		}
	}

but only nWords amount of uint64 is unmarshaled

	nWords := wordsNeeded(uint(length))
	reader := bufio.NewReader(io.LimitReader(stream, 8*int64(nWords)))
	for i := 0; i < nWords; i++ {
		if _, err := io.ReadFull(reader, item[:]); err != nil {
			if err == io.EOF {
				err = io.ErrUnexpectedEOF
			}
			return 0, err
		}
		newset.set[i] = binaryOrder.Uint64(item[:])
	}

this results in UnmarshalBinary consuming only part of the data emitted by MarshalBinary
the remaning part causes all sorts of uncertainty in the deserialization process of complex data structures.

WriteTo should consider b.length to decide how many words it should marshal

@lemire
Copy link
Member

lemire commented Nov 16, 2022

Yes. Pull request invited.

omerfirmak added a commit to omerfirmak/bitset that referenced this issue Nov 17, 2022
omerfirmak added a commit to omerfirmak/bitset that referenced this issue Nov 17, 2022
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