Skip to content

Properly stringify both large and small int64's when converting to json#1060

Merged
vladtr merged 6 commits intomainfrom
stringify-json-fix
Apr 20, 2023
Merged

Properly stringify both large and small int64's when converting to json#1060
vladtr merged 6 commits intomainfrom
stringify-json-fix

Conversation

@vladtr
Copy link
Copy Markdown
Contributor

@vladtr vladtr commented Apr 19, 2023

This PR contains fix (and ut's) for the following issue (#579):

I expected to receive an action value is -90909090909090909. But the action return the value is -90909090909090910

[[eosio::action]] int64_t getvalue2() {
int64_t value = -90909090909090909;
eosio::print("getvalue2: ", value);
return value;
}
You can reproduce this issue through this repo https://github.com/quocle108/return-error

@vladtr vladtr linked an issue Apr 19, 2023 that may be closed by this pull request
@vladtr vladtr added this to the Leap v5.0.0-rc1 milestone Apr 19, 2023
int64_t i = v.as_int64();
if( format == json::output_formatting::stringify_large_ints_and_doubles &&
i > 0xffffffff )
(i * ((i>0)-(i<0)) > 0xffffffff))
Copy link
Copy Markdown
Contributor

@heifner heifner Apr 20, 2023

Choose a reason for hiding this comment

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

I find this construct rather odd and I am a bit surprised that compilers would not give a warning about bool used in arithmetic statement.

Seems like: (i > 0xffffffff) || (i < -0xffffffff) would be clearer.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

how about std::abs(i) > 0xffffffff
probably cmath should be included

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Casting to uint64_t should also do the job, i.e. static_cast<uint64_t>(i) > 0xffffffff

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thank you, all great suggestions! Changed.

@vladtr vladtr merged commit 2b5b3ce into main Apr 20, 2023
@vladtr vladtr deleted the stringify-json-fix branch April 20, 2023 17:53
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.

action return incorrect int64_t value

4 participants