From 66ac38d38181360889d3133e4d41dc0ebca3554e Mon Sep 17 00:00:00 2001 From: Furquan Shaikh Date: Thu, 3 Sep 2020 21:53:17 -0700 Subject: motion_sense: Fix signed/unsigned comparison in ec_motion_sensor_clamp_* helpers ec_motion_sensor_clamp_* helpers are performing comparison between signed and unsigned integers leading to non-compliant code. This results in comparison operation converting negative values to unsigned integers. Example: When ec_motion_sensor_clamp_i16() is called with a value of -15000, it returns 32767 instead of -15000 because: * MAX(-15000, INT16_MIN) ==> MAX(-15000, -32768) ==> -15000 * MIN(-15000, INT16_MAX) ==> MIN(-15000, 32767U) ==> This expands to: int32_t temp_a = -15000; uint32_t temp_b = 32767U; << This is my assumption what the compiler would be doing (temp_a < temp_b) ? temp_a : temp_b; In this comparison, temp_a is signed int whereas temp_b is unsigned int. As per C-standard for usual arithmetic conversions, for the comparison operation, both operands are converted to unsigned integer type corresponding to the type of the operand with signed integer type. Thus the comparison operation changes to comparison between: -15000 --> 0xffffc568 32767 --> 0x7fff and hence returns 0x7fff i.e. 32767. This change adds explicit type cast for constant values passed into MIN()/MAX() to ensure that the comparison is compliant to C standard. With this the above comparison in ec_motion_sensor_clamp_i16() correctly returns -15000. BUG=b:167751183,b:189894836 BRANCH=zork TEST=Verified that screen flips when in tablet mode on Morphius. Also, verified using following steps that raw data seen in sysfs consists of positive and negative values: $ cd /sys/bus/iio/devices $ watch -n 0.1 grep . */in_*_raw Signed-off-by: Furquan Shaikh Change-Id: I444c9aeab4ae8a4726600222080fb141084b7a41 Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/2393459 Tested-by: Furquan Shaikh Reviewed-by: Denis Brockus Commit-Queue: Denis Brockus Auto-Submit: Furquan Shaikh (cherry picked from commit ec743f4b565061208d33dfd36d8fd753197f998c) Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/2932464 Reviewed-by: Gwendal Grignou Commit-Queue: Gwendal Grignou Tested-by: Gwendal Grignou --- include/motion_sense.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/motion_sense.h b/include/motion_sense.h index 08e78bc75c..c340f6b916 100644 --- a/include/motion_sense.h +++ b/include/motion_sense.h @@ -301,7 +301,7 @@ enum motionsensor_orientation motion_sense_remap_orientation( */ static inline uint16_t ec_motion_sensor_clamp_u16(const int32_t value) { - return (uint16_t)MIN(MAX(value, 0), UINT16_MAX); + return (uint16_t)MIN(MAX(value, 0), (int32_t)UINT16_MAX); } static inline void ec_motion_sensor_clamp_u16s(uint16_t *arr, const int32_t *v) { @@ -312,7 +312,7 @@ static inline void ec_motion_sensor_clamp_u16s(uint16_t *arr, const int32_t *v) static inline int16_t ec_motion_sensor_clamp_i16(const int32_t value) { - return MIN(MAX(value, INT16_MIN), INT16_MAX); + return MIN(MAX(value, (int32_t)INT16_MIN), (int32_t)INT16_MAX); } static inline void ec_motion_sensor_clamp_i16s(int16_t *arr, const int32_t *v) { -- cgit v1.2.1