Re: [PATCH v2 0/1] MR6169: comctl32: fixed first we check index then array access by index is safe

[Alfred Agrell (＠Alcaro)]

On Tue Jul 30 08:54:54 2024 +0000, Nikolay Sivov wrote:
> Agreed. We could have a helper to return btnPtr for given index, also
> checking for bounds. Otherwise this change shouldn't make any difference.

I'm nor aware of this kind of stuff causing any trouble in practice, but I agree with fixing it anyways. Creating a two-or-more-past-the-end pointer is UB.

I also agree with refactoring the bounds check into a function instead of having five different copies. (The last two are TOOLBAR_GetButton and TOOLBAR_SetCmdId, which are correct, but still duplicate.)

Finally, I strongly suspect TOOLBAR_GetCheckedGroupButtonIndex contains a similar bug. I suspect the > should be >=.