This repository has been archived by the owner. It is now read-only.

Buffer assertion fix #4447

Closed
wants to merge 3 commits into
from

Conversation

Projects
None yet
1 participant
@trevnorris

The first commit adds a check to write at offset -Infinity.

The second consolidates write assert checks into a set of 3 functions. This also removes a few duplicate checks that existed.

The third consolidates and fixes the assert checks for the read methods.

If the seconds and third commits are too much of a change, just cherry-pick the first.

These changes currently conflict with master. I can port the patch if needed.

@trevnorris

This comment has been minimized.

Show comment Hide comment
@trevnorris

trevnorris Dec 21, 2012

Just in case I forget about this patch, @isaacs this is what was mentioned in irc.

Just in case I forget about this patch, @isaacs this is what was mentioned in irc.

@trevnorris

This comment has been minimized.

Show comment Hide comment
@trevnorris

trevnorris Dec 24, 2012

Bugger. Just read up on issue #3934. The extended patch will break the master branch. I'll create a fix and rebase the changes.

Bugger. Just read up on issue #3934. The extended patch will break the master branch. I'll create a fix and rebase the changes.

@trevnorris

This comment has been minimized.

Show comment Hide comment
@trevnorris

trevnorris Jan 2, 2013

@isaacs I fixed the forward compatibility breakage and rebased on latest v0.8. Should be ready for consumption. Let me know whether you'll just cherry pick the first commit or if I should squash them all.

@isaacs I fixed the forward compatibility breakage and rebased on latest v0.8. Should be ready for consumption. Let me know whether you'll just cherry pick the first commit or if I should squash them all.

trevnorris added some commits Dec 20, 2012

buffer: fail verbosely on all bad offset values
offset === -Infinity used to fail silently
buffer: consolidate write assert tests
Assert tests for all write methods have been consolidated to a set of
three functions that can be called as needed. This prevents unnecessary
duplication of assert checks.
buffer: consolidate read assert tests
Incorrect offset values could still get past pre-existing assert checks.
They have been updated to prevent this from happening. Also have been
combine into a single arguments check function.
@trevnorris

This comment has been minimized.

Show comment Hide comment
@trevnorris

trevnorris Jan 15, 2013

Already have another patch for review that fixes everything done here, and much better.

Already have another patch for review that fixes everything done here, and much better.

@trevnorris trevnorris closed this Jan 15, 2013

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.