charleslemaire0 / TeenAstro

an easy to build and use telescope controller
GNU General Public License v3.0
24 stars 21 forks source link

Fixed potential bug in `atoi2` #45

Closed dssgabriel closed 1 year ago

dssgabriel commented 1 year ago

Description

This pull request fixes a potential bug when checking the value of the converted string in atoi2 and updates the frac function.

Value checking in atoi2()/atoui2()

The previous code checked that the value of l was in the range of $\left[-32767,+32768\right]$.
This is wrong because on a 32-bit system such as the Teensy, the C99/C++11 standard defines that the int type is stored on 16 bits and that long is stored on 32 bits. Therefore, when checking the value of l, we must ensure that it is in the range of a 16-bit integer, which is $\left[-2^{15},+2^{15}-1\right]$, i.e $\left[-32768,+32767\right]$.
In the edge case where l had the value of 32768, the condition in the if statement would evaluate as false, causing the function to return true and update i with a value of -32768 (see two's complement representation of signed integers to understand why).

This potential bug was removed by using the explicit-sized types defined in the <cstdint> header (int16_t, uint32_t, etc...) and their defined limit values (INT16_MIN, UINT16_MAX, etc...). To ensure that l is correctly downcasted to an int16_t when updating the value of i, an explicit type conversion was added.
For the sake of homogeneity, this was also replicated in the atoui2 function (although the limit value used was already correct).

Correct floating-point arithmetic in frac()

The frac() function was also updated to replace the integer cast with a call to the floor() function. This ensures that there is no implicit type conversion and correct floating-point arithmetic.