Fix Miscasting [API-2378] [HZ-5417]#1433
Conversation
emreyigit
changed the title
ihsandemir
left a comment
ihsandemir
left a comment
There was a problem hiding this comment.
can you add a test to verify it was actually a problem?
ihsandemir
left a comment
ihsandemir
left a comment
There was a problem hiding this comment.
Can you also add a non-trivial array which causes infinite loop without this fix but works after this fix? This will validate the fix.
|
|
||
| TEST_F(DecimalTest, twos_complement_multi_byte_full_carry) | ||
| { | ||
| // carry propagates out of every byte; a size_t loop index would wrap to |
There was a problem hiding this comment.
what does this comment mean? you changed size_t usage to int int he loop at this PR.
There was a problem hiding this comment.
Yes, only way to test it is calling the function and verifying its result. I just explained the situation what was it before. Also, I couldn't find any test on twos_complement, so I added single and multiple vector size test which verifies both loop and the function.
There was a problem hiding this comment.
Actually, there were existing tests for the complement (see DecimalTest::.... test cases above your added test. e.g. TEST_F(DecimalTest, carry_bit_test)) but not for this exact scenario. you need a specific value to actually see the index goes all the way to 0. Your added tests are OK but they do not catch the infinite loop problem, right? Does you test catch the failure without the fix?
There was a problem hiding this comment.
The test should verify the loop as well since it was discarding the carry bit before due to unsigned int casting.
2 participants
Refactored the casting.
closes #1432