Re: [patch] fbdev: off by one test (harmless)

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 14/01/15 07:24, David Ung wrote:
> 
>> -----Original Message-----
>> From: Tomi Valkeinen [mailto:tomi.valkeinen@xxxxxx]
>> Sent: Tuesday, January 13, 2015 3:19 AM
>> To: David Ung; 'Jingoo Han'; 'Dan Carpenter'
>> Cc: 'Jean-Christophe Plagniol-Villard'; 'Daniel Vetter'; 'Laurent Pinchart';
>> 'Rob Clark'; linux-fbdev@xxxxxxxxxxxxxxx; kernel-janitors@xxxxxxxxxxxxxxx
>> Subject: Re: [patch] fbdev: off by one test (harmless)
>>
>> * PGP Signed by an unknown key
>>
>> On 29/12/14 23:51, David Ung wrote:
>>>
>>>>> test is really a no-op.  But it's still off by one and upsets the
>>>>> static checkers so we may as well correct it.
>>>>
>>>> 'idx' should be 0~63, because cea_modes array is defined as
>>>> 'cea_modes[64]'.
>>>
>>> According to CEA/EIA-861E, there are 64 defined modes, but idx is
>>> valid for 1-64, 0 is reserved hence the check for
>>>
>>>         If (!idx || idx > 63) {
>>>
>>> Think idx check really should be !idx || idx > 64 if following
>>> CEA/EIA-861E
>>
>> In that case there's something funny with the code. The code indexes
>> 'cea_modes' using 'idx', and I _think_ cea_modes is already offset properly,
>> i.e. there's no entry at 0. But its length is 64, which is not right, as there's the
>> empty item in the beginning.
>>
>> So maybe the correct fix is to increase the length of cea_modes to 65, and
>> change the idx check as you mention above?
>>
> 
> In that case might aswell go with CEA/EAI-861F for completeness and have the check against 107.
> but with a slight waste of memory.

Then we need to fill in the new modes. Or leave them blank, but then,
what would be the point.

And I'd rather not add new stuff to fbdev that is not absolutely needed.
Everybody should move to DRM =).

 Tomi


Attachment: signature.asc
Description: OpenPGP digital signature


[Index of Archives]     [Video for Linux]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Tourism]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux