diff options
| author | Furquan Shaikh <furquan@google.com> | 2020-09-03 21:53:17 -0700 |
|---|---|---|
| committer | Commit Bot <commit-bot@chromium.org> | 2020-09-05 01:01:01 +0000 |
| commit | ec743f4b565061208d33dfd36d8fd753197f998c (patch) | |
| tree | bc03c6a0da4cb22b118dd73a974bb2efc6eaebb9 /include/motion_sense.h | |
| parent | fd3ee1992ac797473644928e8ff15543057f37e6 (diff) | |
| download | chrome-ec-ec743f4b565061208d33dfd36d8fd753197f998c.tar.gz | |
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
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 <furquan@google.com>
Change-Id: I444c9aeab4ae8a4726600222080fb141084b7a41
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/2393459
Tested-by: Furquan Shaikh <furquan@chromium.org>
Reviewed-by: Denis Brockus <dbrockus@chromium.org>
Commit-Queue: Denis Brockus <dbrockus@chromium.org>
Auto-Submit: Furquan Shaikh <furquan@chromium.org>
Diffstat (limited to 'include/motion_sense.h')
| -rw-r--r-- | include/motion_sense.h | 4 |
1 files changed, 2 insertions, 2 deletions
diff --git a/include/motion_sense.h b/include/motion_sense.h index d30ff9d7ac..f6a4911239 100644 --- a/include/motion_sense.h +++ b/include/motion_sense.h @@ -337,7 +337,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) { @@ -348,7 +348,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) { |