Re: [PATCH] readkey: shrink table of known escape sequences in size

[Ahmad Fatoum <a.fatoum@pengutronix.de>]

Hi,

On 9/28/20 1:58 PM, Sascha Hauer wrote:
>>>>  	if (c == 27) {
>>>> @@ -67,7 +69,7 @@ int read_key(void)
>>>>  		}
>>>>  		esc[i] = 0;
>>>>  		for (i = 0; i < ARRAY_SIZE(esccmds); i++){
>>>> -			if (!strcmp(esc, esccmds[i].seq))
>>>> +			if (!strncmp(esc, esccmds[i].seq, MAX_ESC_LEN))
>>>>  				return esccmds[i].val;
>>>
>>> Anyway, I don't think we should play tricks with dropping string
>>> termination characters just to squeeze some bytes out of the binary.
>>
>> I can define #define MAX_ESC_LEN 4 if you prefer that, but I consider
>> it superfluous.
> 
> That would allow you to use strcmp and I really prefer that. strncmp
> doesn't give you the desired result when one string is longer than the
> length given in the third argument. Proving that this cannot happen here
> requires more brain time than parsing strcmp, at least in my brain ;)

I would still maintain the strncmp, because it's less error-prone:
If in future a new sequence is added, it will be truncated instead of
an out-of-bounds read.

v2 just sent out

> 
> Sascha
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox