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.

Sign up for GitHub

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

Jump to bottom

Backport 7176 for v6.x (buffer: fix creating from zero-length ArrayBuffer) #7421

Closed
wants to merge 1 commit into from
Closed

Backport 7176 for v6.x (buffer: fix creating from zero-length ArrayBuffer) #7421

wants to merge 1 commit into from

Conversation

RReverser
Copy link
Member

@RReverser RReverser commented Jun 25, 2016 •
edited
Loading

Checklist
  • make -j4 test (UNIX) or vcbuild test nosign (Windows) passes
  • a test and/or benchmark is included
  • the commit message follows commit guidelines
Affected core subsystem(s)

buffer

Description of change

This is a backport of #7176 for v6.x. Applied cleanly.

@RReverser
buffer: fix creating from zero-length ArrayBuffer
c2e0b5f 
Fixes regression where creating a new Buffer from an
empty ArrayBuffer would fail.

Ref: 85ab4a5
PR-URL: #7176
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Сковорода Никита Андреевич <[email protected]>
Reviewed-By: Ron Korving <[email protected]>
@nodejs-github-bot nodejs-github-bot added the buffer Issues and PRs related to the buffer subsystem. label Jun 25, 2016
@addaleax addaleax added the v6.x label Jun 25, 2016
@RReverser
Copy link
Member Author

CI: https://ci.nodejs.org/job/node-test-pull-request/3087/

@Fishrock123
Copy link
Contributor

@RReverser Does the original not land cleanly? If it landed cleanly, back-porting is not necessary.

@RReverser
Copy link
Member Author

Hmm. It did for me, but @evanlucas wrote in the original PR #7176 (comment):

This is not landing cleanly on v6.x. Going to hold off for now. @RReverser interested in opening a backport PR targeting the v6.x branch?

@evanlucas
Copy link
Contributor

Interesting, I know for a fact none of the buffer ones landed cleanly, but they could have been depending on something else that has been pulled in now?

@RReverser
Copy link
Member Author

Interesting, I know for a fact none of the buffer ones landed cleanly, but they could have been depending on something else that has been pulled in now?

Perhaps this one couldn't be landed exactly because of dependency on #7349 which didn't apply 100% cleanly, but now that it's merged, this one should be fine.

@evanlucas
Copy link
Contributor

@RReverser that makes sense.

@RReverser
Copy link
Member Author

Anyway, I'm happy to either proceed with this PR or close it, just let me know, which way is simpler and preferable :)

@evanlucas
Copy link
Contributor

This lgtm, or we could just cherry-pick if it lands cleanly

@RReverser
Copy link
Member Author

It does, so perhaps cherry-pick will be even easier to do.

@evanlucas
Copy link
Contributor

I was able to cherry-pick this cleanly on v6.x. Thanks for going through the trouble of doing this though @RReverser!

@evanlucas evanlucas closed this Jul 18, 2016
@RReverser
Copy link
Member Author

@evanlucas No problem at all, thanks for doing that!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
buffer Issues and PRs related to the buffer subsystem.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@RReverser @Fishrock123 @evanlucas @addaleax @nodejs-github-bot