10000 Fix ABI breakage by bnoordhuis · Pull Request #503 · nodejs/http-parser · GitHub
[go: up one dir, main page]
Skip to content
This repository was archived by the owner on Nov 6, 2022. It is now read-only.

Fix ABI breakage #503

Closed
wants to merge 0 commits into from
Closed

Fix ABI breakage #503

wants to merge 0 commits into from

Conversation

bnoordhuis
Copy link
Member

Fix ABI breakage introduced in commit 7d5c99d ("Support multi-coding
Transfer-Encoding") by undoing the change in sizeof(http_parser).

Restore the size of the flags field and shrink the index field from
7 to 5 bits. It track strings up to strlen("Transfer-Encoding") bytes
in size so 2^5 == 32 is wide enough.

Fixes: #502

@bnoordhuis bnoordhuis mentioned this pull request Mar 23, 2020
Closed
mcollina
mcollina approved these changes Mar 23, 2020
View reviewed changes
Copy link
Member
@mcollina mcollina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

indutny
indutny approved these changes Mar 23, 2020
View reviewed changes
Copy link
Member
@indutny indutny left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Nice reuse of the fields!

@maxcrees
Copy link

This doesn't resolve the problem with enum http_errno. I think INVALID_TRANSFER_ENCODING needs to be moved to the end of HTTP_ERRNO_MAP or something.

@bnoordhuis
Copy link
Member Author

Good call, I forgot to update that one. Done in a fix-up commit.

bnoordhuis added a commit that referenced this pull request Mar 24, 2020
@bnoordhuis
Fix ABI breakage
714cbb2 
Fix ABI breakage introduced in commit 7d5c99d ("Support multi-coding
Transfer-Encoding") by undoing the change in `sizeof(http_parser)`.

Restore the size of the `flags` field and shrink the `index` field from
7 to 5 bits. It track strings up to `strlen("Transfer-Encoding")` bytes
so 2^5 == 32 is wide enough.

Fixes: #502
PR-URL: #503
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
@bnoordhuis bnoordhuis closed this Mar 24, 2020
@bnoordhuis bnoordhuis deleted the fix502 branch March 24, 2020 10:57
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@sam-github sam-github sam-github approved these changes

@mcollina mcollina mcollina approved these changes

@indutny indutny indutny approved these changes

Assignees
No one assigned
Labels
None yet
< 5070 div class="discussion-sidebar-item js-discussion-sidebar-item js-socket-channel js-updatable-content" data-channel="eyJjIjoicHVsbF9yZXF1ZXN0OjM5MjMzODIzOSIsInQiOjE3NTAyNjcyMDB9--dad0bf4fe3c795b6efb8988e437e084bdcdb74f7a1b064a5d3ec0c4cb1f4676d" data-gid="MDExOlB1bGxSZXF1ZXN0MzkyMzM4MjM5" data-url="/nodejs/http-parser/issues/503/show_partial?partial=issues%2Fsidebar%2Fshow%2Fprojects" data-channel-event-name="projects_updated" >
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

2.9.3 breaks ABI compatibility with 2.9.2 with no corresponding SONAME change
5 participants
@bnoordhuis @maxcrees @sam-github @mcollina @indutny
0