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

js_stream: fix buffer index in DoWrite #1635

Closed
wants to merge 1 commit into from
Closed

js_stream: fix buffer index in DoWrite #1635

wants to merge 1 commit into from

Conversation

shigeki
Copy link
Contributor

@shigeki shigeki commented May 6, 2015

The index of buffer to write in JSStream was always 0 by mistake. This fix was to use increment index of buffer arrays. The test was originally made by @mscdex in #1594.

CI results are https://jenkins-iojs.nodesource.com/job/iojs+any-pr+multi/646/ . Most of failures are caused by cpplint errors.

Fixes: #1595
Fixes: #1594

R= @indutny

js_stream: fix buffer index in DoWrite
d1343d7 
The index of buffer to write in JSStream was always 0 by mistake. This
fix was to use increment index of buffer arrays.
The test was originally made by Brian White in nodejs#1594.

Fixes: nodejs#1595
Fixes: nodejs#1594
@mscdex mscdex added tls Issues and PRs related to the tls subsystem. c++ Issues and PRs that require attention from people who are familiar with C++. stream Issues and PRs related to the stream subsystem. labels May 6, 2015
@indutny
Copy link
Member

indutny commented May 6, 2015

LGTM, thank you!

shigeki pushed a commit that referenced this pull request May 6, 2015
@indutny
js_stream: fix buffer index in DoWrite
6ccbe75 
The index of buffer to write in JSStream was always 0 by mistake. This
fix was to use increment index of buffer arrays.
The test was originally made by Brian White in #1594.

Fix: #1595
Fix: #1594
PR-URL: #1635
Reviewed-By: Fedor Indutny <fedor@indutny.com>
@indutny
Copy link
Member

indutny commented May 6, 2015

Landed in 6ccbe75, thank you @shigeki !

@indutny indutny closed this May 6, 2015
This was referenced May 6, 2015
Closed
Closed
@rvagg rvagg mentioned this pull request May 7, 2015
Closed
Fishrock123 pushed a commit to Fishrock123/node that referenced this pull request May 19, 2015
@Fishrock123
js_stream: fix buffer index in DoWrite
46561fc 
The index of buffer to write in JSStream was always 0 by mistake. This
fix was to use increment index of buffer arrays.
The test was originally made by Brian White in nodejs#1594.

Fix: nodejs#1595
Fix: nodejs#1594
PR-URL: nodejs#1635
Reviewed-By: Fedor Indutny <fedor@indutny.com>
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
c++ Issues and PRs that require attention from people who are familiar with C++. stream Issues and PRs related to the stream subsystem. tls Issues and PRs related to the tls subsystem.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

tls: OpenSSL errors with many tls socket writes
3 participants
@shigeki @indutny @mscdex