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

Load: Fix signed integer limit error. #179

Merged
merged 1 commit into from
Jan 27, 2022
Merged

Conversation

a2mpt
Copy link

@a2mpt a2mpt commented Jan 26, 2022

When loading signed integer in cyaml__read_int(), the min to max range is set to only half of the supposed value:

  • int8_t: -64 to 63
  • short: -16384 to 16383
  • int: -2^30 to 2^30-1
  • long: -2^62 to 2^62-1

So if the data type is short, a value like 20000 or -20000 in the YAML file will cause:
libcyaml: ERROR: Load: Invalid INT value: '20000'

Should use UINT64_MAX instead of INT64_MAX in the shift, like that in this pull request. Another solution is do not divide the value by 2:
max = (INT64_MAX >> ((8 - schema->data_size) * 8));

@codecov
Copy link

codecov bot commented Jan 27, 2022

Codecov Report

Merging #179 (2e233c6) into main (5206ece) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #179   +/-   ##
=======================================
  Coverage   97.13%   97.13%           
=======================================
  Files           9        9           
  Lines        1608     1608           
  Branches      347      347           
=======================================
  Hits         1562     1562           
  Misses         24       24           
  Partials       22       22           
Flag Coverage Δ
unittests 97.13% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/load.c 99.20% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5206ece...2e233c6. Read the comment docs.

@tlsa
Copy link
Owner

tlsa commented Jan 27, 2022

Splendid! Good spot, thanks! 😸

@tlsa tlsa merged commit 99c4cb5 into tlsa:main Jan 27, 2022
@a2mpt a2mpt deleted the signed-integer-limit branch January 27, 2022 12:28
@tlsa
Copy link
Owner

tlsa commented Feb 1, 2022

Now released in v1.3.1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants