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.

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

Fix typed json encoder conversion from scalar's PV slot to JSON_TYPE_INT #133

Closed
wants to merge 1 commit into from

Conversation

pali
Copy link
Contributor

@pali pali commented Apr 17, 2019

It did not worked correctly for numbers which can be represented by
unsigned type but not by signed type: [2^(8*IVSIZE-1), 2^(8*IVSIZE)-1]

SvIV and SvUV macros may modify SvIsUV flag, so after calling SvIV it is
needed to check if returned value is signed or unsigned.

This patch fixes it and add tests for scalars with corner cases in PV slot.

Values out of range of perl integers are converted for JSON_TYPE_INT to
either IV_MIN or UV_MAX (signed minimal or unsigned maximal represented
value in perl).

It did not worked correctly for numbers which can be represented by
unsigned type but not by signed type: [2^(8*IVSIZE-1), 2^(8*IVSIZE)-1]

SvIV and SvUV macros may modify SvIsUV flag, so after calling SvIV it is
needed to check if returned value is signed or unsigned.

This patch fixes it and add tests for scalars with corner cases in PV slot.

Values out of range of perl integers are converted for JSON_TYPE_INT to
either IV_MIN or UV_MAX (signed minimal or unsigned maximal represented
value in perl).
@rurban
Copy link
Owner

rurban commented Apr 17, 2019

Looks good, thanks, but I'm still working on the smokers

@rurban rurban self-requested a review April 17, 2019 08:37
@rurban rurban self-assigned this Apr 17, 2019
@rurban rurban added the bug label Apr 17, 2019
@rurban
Copy link
Owner

rurban commented Apr 17, 2019

Merged with 9d3012f and 8fee7c5. And yes, I quoted the user-controlled path. Thanks!

@rurban rurban closed this Apr 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants