> On 16 Mar 2020, at 09:49, Stefan Sperling <s...@stsp.name> wrote:
>
> On Mon, Mar 16, 2020 at 09:29:43AM +0100, Jasper Lievisse Adriaanse wrote:
>> Hi,
>>
>> The type of brightness and video_output is uint32_t; therefore it
>> can never be less than 0 (which is what HCI_LCD_BRIGHTNESS_MIN and
>> HCI_VIDEO_OUTPUT_CYCLE_MIN are defined to). So trim the checks by
>> removig the impossible cases.
>>
>> Coverity CID 1453109, 1453169
>>
>> OK?
>
> These values ultimately come from aml_val2int() which returns int64_t,
> i.e. a signed value. This driver currently copies that value to a uint32_t,
> shifts it, and this shifted value ends up being copied to an int, and then
> again to a uint32_t, which is then range-checked.
> I don't understand why a range check is done only that late in the game,
> after multiple integer type conversions that seem to be performed for no
> good reason other than that the expected range will fit into an int.
>
> So an alternative fix could be to switch brightness values in this
> driver to int64_t everywhere, i.e. in toshiba_get_brightness(),
> toshiba_find_brightness(), toshiba_set_brightness(), etc.
> Then you can keep all the code as it is.
>
> Or read an int64_t with aml_val2int(), shift and range-check it right away,
> and only convert to a narrower type if the value is in the expected range.
> Else error.
Right, it seems that other drivers either use an uint64_t with associated
shifts or
int to match wsdisplay_params->curval.
Also, it seems this passing around of ‘brightness’ could be avoided by making
it a member
of acpitoshiba_softc; same goes for 'video_output'.
