Skip to content
This repository was archived by the owner on Nov 6, 2022. It is now read-only.

Fix ABI breakage#503

Closed
bnoordhuis wants to merge 0 commit into
nodejs:masterfrom
bnoordhuis:fix502
Closed

Fix ABI breakage#503
bnoordhuis wants to merge 0 commit into
nodejs:masterfrom
bnoordhuis:fix502

Conversation

@bnoordhuis

Copy link
Copy Markdown
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

@mcollina mcollina left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

lgtm

@indutny indutny left a comment

Copy link
Copy Markdown
Member

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
Copy Markdown

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
Copy Markdown
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
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 <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Sam Roberts <[email protected]>
@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.

Labels

None yet

Projects

None yet

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