[PATCH 1/5] comctl32/listbox: Check for out of bounds index using an unsigned index

[Huw Davies]

On Thu, Feb 07, 2019 at 01:13:36PM +0200, Gabriel Ivăncescu wrote:
> On 2/7/19 11:52 AM, Huw Davies wrote:
> > On Thu, Jan 31, 2019 at 05:23:19PM +0200, Gabriel Ivăncescu wrote:
> > > Signed-off-by: Gabriel Ivăncescu <gabrielopcode at gmail.com>
> > > ---
> > >   dlls/comctl32/listbox.c | 4 ++--
> > >   1 file changed, 2 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/dlls/comctl32/listbox.c b/dlls/comctl32/listbox.c
> > > index cb645b4..71f1c05 100644
> > > --- a/dlls/comctl32/listbox.c
> > > +++ b/dlls/comctl32/listbox.c
> > > @@ -489,7 +489,7 @@ static INT LISTBOX_GetItemFromPoint( const LB_DESCR *descr, INT x, INT y )
> > >    * Paint an item.
> > >    */
> > >   static void LISTBOX_PaintItem( LB_DESCR *descr, HDC hdc, const RECT *rect,
> > > -			       INT index, UINT action, BOOL ignoreFocus )
> > > +			       UINT index, UINT action, BOOL ignoreFocus )
> > 
> > Could you leave this change out?  descr->nb_items itself is signed as is the index
> > parameter of LISTBOX_RePaintItem().
> > 
> 
> Hi Huw,
> 
> We have to make it unsigned because checking for bounds then only really
> needs one comparison, as Nikolay suggested awhile ago. And the bounds check
> will be needed for LBS_NODATA later (can't test for NULL as that would be
> wrong when the item is 0 and nodata).

How does LISTBOX_PaintItem() get called with index < 0 ?  If it did, we'd
already be in trouble.

My point is really that if we're going to fix the signess of index, we need to
go through and fix it properly in all the code, not just one parameter at a
time.  And then it should be a separate patch.

Huw.