Skip to content

Fix dynamic var conversion precision check (#4886) #4894

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Mar 11, 2025
Merged

Fix dynamic var conversion precision check (#4886) #4894

merged 3 commits into from
Mar 11, 2025

Conversation

matejk
Copy link
Contributor

@matejk matejk commented Mar 11, 2025

Fixes #4886, replaces #4887.

@matejk matejk added this to the Release 1.14.2 milestone Mar 11, 2025
@matejk matejk added this to 1.14 Mar 11, 2025
@matejk matejk mentioned this pull request Mar 11, 2025
Closed
@matejk matejk self-assigned this Mar 11, 2025
@matejk matejk moved this to In Progress in 1.14 Mar 11, 2025
github-advanced-security[bot]
github-advanced-security bot found potential problems Mar 11, 2025
View reviewed changes
@matejk matejk force-pushed the 4886-fix-dynamic-var-conversion-precision-check branch from 48b5403 to 2d129c1 Compare March 11, 2025 09:45
@matejk @github-advanced-security
Potential fix for code scanning alert no. 1075: Declaration hides var…
56ed979 
…iable

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
@matejk matejk force-pushed the 4886-fix-dynamic-var-conversion-precision-check branch from 318c4ff to 56ed979 Compare March 11, 2025 09:50
@matejk matejk merged commit 91244ac into main Mar 11, 2025
47 checks passed
@matejk matejk deleted the 4886-fix-dynamic-var-conversion-precision-check branch March 11, 2025 12:29
@matejk matejk moved this from In Progress to Done in 1.14 Mar 11, 2025
matejk added a commit that referenced this pull request Mar 11, 2025
@matejk @MduPlessis-1 @github-advanced-security
Fix dynamic var conversion precision check (#4886) (#4894)
66399fa 
* Take the absolute value of the value in numValDigits to fix the precision loss check for negative values.

* fix(VarHolder): Correct numValDigits to properly convert to signed integer.

* Potential fix for code scanning alert no. 1075: Declaration hides variable

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>

---------

Co-authored-by: Mias <mias@innoventix.co.za>
Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
@aleks-f
Copy link
Member

aleks-f commented Mar 19, 2025

this is causing warnings on windows:

C:\Users\alex\source\repos\pocoproject\poco\Foundation\include\Poco\Dynamic\VarHolder.h(478,51): warning C4146: unary minus operator applied to unsigned type, result still unsigned
1>(compiling source file 'src/JSONConfiguration.cpp')
1>    C:\Users\alex\source\repos\pocoproject\poco\Foundation\include\Poco\Dynamic\VarHolder.h(478,51):
1>    the template instantiation context (the oldest one first) is
1>        C:\Users\alex\source\repos\pocoproject\poco\Foundation\include\Poco\Dynamic\VarHolder.h(2124,3):
1>        see reference to function template instantiation 'void Poco::Dynamic::VarHolder::convertToFP<const Poco::UInt32,float,true,true>(F &,T &)' being compiled
1>        with
1>        [
1>            F=const Poco::UInt32,
1>            T=float
1>        ]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@matejk matejk

Labels
None yet
Projects
1.14
Status: Done
Milestone
Release 1.14.2
Development

Successfully merging this pull request may close these issues.

Poco::Dynamic::Var conversion to floating point does not handle precision checks correctly for negative numbers.
3 participants
@matejk @aleks-f @MduPlessis-1