search

OnePlusOSS / android_kernel_oneplus_sm8350

Other
64 stars 50 forks source link

Code error in oplus_chg_comm.c #4

Open kristofpetho opened 3 years ago

kristofpetho commented 3 years ago

https://github.com/OnePlusOSS/android_kernel_oneplus_sm8350/blob/2206c099f741005fb38dd941455f78eecf4a6104/techpack/oneplus/power/oplus_chg/oplus_chg_comm.c#L1436

Hi @YuHuang65, should this not be "FFC_TEMP_INVALID" instead of "BATT_TEMP_INVALID"?

Regards,

Kristof

YuHuang65 commented 3 years ago

I just check the codes there, it is actually "BATT_TEMP_INVALID" .

Ionic commented 2 years ago

Can you please explain why you think that BATT_TEMP_INVALID is valid there?

comm_dev->ffc_temp_dynamic_thr is a structure of size FFC_TEMP_INVALID - 1. Currently, BATT_TEMP_INVALID is bigger than FFC_TEMP_INVALID, so once the loop reaches the FFC_TEMP_INVALID value (currently 4, but the specifics don't really matter), it'll be overwriting later fields like possibly temp_region, ffc_temp_region and part of the comm_cfg field/structure (I'd have to check alignment rules on aarch64 to find out specifics).

The initial reporter's request sounds sane, unless I missed something critical.

kristofpetho commented 2 years ago

Can you please explain why you think that BATT_TEMP_INVALID is valid there?

comm_dev->ffc_temp_dynamic_thr is a structure of size FFC_TEMP_INVALID - 1. Currently, BATT_TEMP_INVALID is bigger than FFC_TEMP_INVALID, so once the loop reaches the FFC_TEMP_INVALID value (currently 4, but the specifics don't really matter), it'll be overwriting later fields like possibly temp_region, ffc_temp_region and part of the comm_cfg field/structure (I'd have to check alignment rules on aarch64 to find out specifics).

The initial reporter's request sounds sane, unless I missed something critical.

That was exactly my thinking. It was actually caught by GCC as an error...