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 iterration problem for non decimal string #2400

Merged
merged 1 commit into from May 26, 2022

Conversation

thalman
Copy link
Contributor

@thalman thalman commented Jan 20, 2022

When the string transformation to number failed, all following
transformation failed too.

This happened because the status in decNumberFromString function is
updated just in case of error. See the line

 if (status!=0) decStatus(dn, status, set);

Reusing the DEC_CONTEXT that failed before results into error even if the string is valid number.

I realize that this is not good solution. This PR is meant more to start the discussion on how to fix the issue correctly. I'm not familiar with the jq code.

With this patch all tests works. So maybe just making decStatus() available and calling it here would be good enough. Maybe setting the status to 0 at the beginning of decNumberFromString()?

What do you think?

@thalman
Copy link
Contributor Author

thalman commented Feb 1, 2022

Fixes Issue #2406

When the string transformation to number failed, all following
transformation failed too.

This happend because status in decNumberFromString function is
updated just in error case. Reusing the DEC_CONTEXT that failed
before results into error even if the string is valid number.
@thalman
Copy link
Contributor Author

thalman commented Feb 3, 2022

I force pushed small change. Using

decContextClearStatus(ctx, DEC_Conversion_syntax);

is probably better than

 ctx->status = 0;

@thalman
Copy link
Contributor Author

thalman commented May 25, 2022

Fixes issue #2400 too.

@nicowilliams
Copy link
Contributor

Thank you!!!

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