[Mesa-dev] [PATCH] egl: use unsigned int index when iterating over attrib_list

[Eric Engestrom]

On Tue, Sep 27, 2016 at 04:10:53PM +0200, Nicolai Hähnle wrote:
> On 27.09.2016 14:40, Emil Velikov wrote:
> > From: Emil Velikov <emil.velikov at collabora.com>
> > 
> > Otherwise one can overflow the signed variable and (attempt to) cause
> > all sorts of strange behaviour.
> 
> As long as we're worrying about such things, shouldn't it really be a size_t
> then? With that,

Agreed, and you can also have my r-b.

One question though: why these specific `i`s? There are plenty more `i`s
(in these files) that could use the same treatment, not to mention other
variables.
It's not as if these are the most overflow-critical either: I'm pretty
sure if we have >INT_MAX attributes, we have more pressing problems than
overflowing the attrib counter :P
(To be clear, I do think this is a good change, just wondering why)

> 
> Reviewed-by: Nicolai Hähnle <nicolai.haehnle at amd.com>
> 
> Cheers,
> Nicolai
> 
> > 
> > Signed-off-by: Emil Velikov <emil.velikov at collabora.com>
> > ---
> >  src/egl/drivers/dri2/egl_dri2.c | 2 +-
> >  src/egl/main/eglconfig.c        | 3 ++-
> >  src/egl/main/eglcontext.c       | 3 ++-
> >  src/egl/main/egldisplay.c       | 2 +-
> >  src/egl/main/eglimage.c         | 3 ++-
> >  src/egl/main/eglsurface.c       | 3 ++-
> >  src/egl/main/eglsync.c          | 6 ++++--
> >  7 files changed, 14 insertions(+), 8 deletions(-)
> > 
> > diff --git a/src/egl/drivers/dri2/egl_dri2.c b/src/egl/drivers/dri2/egl_dri2.c
> > index 8e376e3..6a3318b 100644
> > --- a/src/egl/drivers/dri2/egl_dri2.c
> > +++ b/src/egl/drivers/dri2/egl_dri2.c
> > @@ -167,7 +167,7 @@ dri2_add_config(_EGLDisplay *disp, const __DRIconfig *dri_config, int id,
> >     _EGLConfig *matching_config;
> >     EGLint num_configs = 0;
> >     EGLint config_id;
> > -   int i;
> > +   unsigned int i;
> > 
> >     dri2_dpy = disp->DriverData;
> >     _eglInitConfig(&base, disp, id);
> > diff --git a/src/egl/main/eglconfig.c b/src/egl/main/eglconfig.c
> > index 6161d26..b12ff9d 100644
> > --- a/src/egl/main/eglconfig.c
> > +++ b/src/egl/main/eglconfig.c
> > @@ -514,7 +514,8 @@ EGLBoolean
> >  _eglParseConfigAttribList(_EGLConfig *conf, _EGLDisplay *dpy,
> >                            const EGLint *attrib_list)
> >  {
> > -   EGLint attr, val, i;
> > +   EGLint attr, val;
> > +   unsigned int i;
> > 
> >     _eglInitConfig(conf, dpy, EGL_DONT_CARE);
> > 
> > diff --git a/src/egl/main/eglcontext.c b/src/egl/main/eglcontext.c
> > index 60625f6..694f137 100644
> > --- a/src/egl/main/eglcontext.c
> > +++ b/src/egl/main/eglcontext.c
> > @@ -85,7 +85,8 @@ _eglParseContextAttribList(_EGLContext *ctx, _EGLDisplay *dpy,
> >                             const EGLint *attrib_list)
> >  {
> >     EGLenum api = ctx->ClientAPI;
> > -   EGLint i, err = EGL_SUCCESS;
> > +   EGLint err = EGL_SUCCESS;
> > +   unsigned int i;
> > 
> >     if (!attrib_list)
> >        return EGL_SUCCESS;
> > diff --git a/src/egl/main/egldisplay.c b/src/egl/main/egldisplay.c
> > index 3d4eb81..201cf7b 100644
> > --- a/src/egl/main/egldisplay.c
> > +++ b/src/egl/main/egldisplay.c
> > @@ -474,7 +474,7 @@ _eglUnlinkResource(_EGLResource *res, _EGLResourceType type)
> >  static EGLBoolean
> >  _eglParseX11DisplayAttribList(const EGLint *attrib_list)
> >  {
> > -   int i;
> > +   unsigned int i;
> > 
> >     if (attrib_list == NULL) {
> >        return EGL_TRUE;
> > diff --git a/src/egl/main/eglimage.c b/src/egl/main/eglimage.c
> > index 818b597..44dbfab 100644
> > --- a/src/egl/main/eglimage.c
> > +++ b/src/egl/main/eglimage.c
> > @@ -41,7 +41,8 @@ EGLint
> >  _eglParseImageAttribList(_EGLImageAttribs *attrs, _EGLDisplay *dpy,
> >                           const EGLint *attrib_list)
> >  {
> > -   EGLint i, err = EGL_SUCCESS;
> > +   EGLint err = EGL_SUCCESS;
> > +   unsigned int i;
> > 
> >     (void) dpy;
> > 
> > diff --git a/src/egl/main/eglsurface.c b/src/egl/main/eglsurface.c
> > index 231a5f0..37ede3e 100644
> > --- a/src/egl/main/eglsurface.c
> > +++ b/src/egl/main/eglsurface.c
> > @@ -70,9 +70,10 @@ _eglParseSurfaceAttribList(_EGLSurface *surf, const EGLint *attrib_list)
> >     _EGLDisplay *dpy = surf->Resource.Display;
> >     EGLint type = surf->Type;
> >     EGLint texture_type = EGL_PBUFFER_BIT;
> > -   EGLint i, err = EGL_SUCCESS;
> > +   EGLint err = EGL_SUCCESS;
> >     EGLint attr = EGL_NONE;
> >     EGLint val = EGL_NONE;
> > +   unsigned int i;
> > 
> >     if (!attrib_list)
> >        return EGL_SUCCESS;
> > diff --git a/src/egl/main/eglsync.c b/src/egl/main/eglsync.c
> > index 33625e9..df313cb 100644
> > --- a/src/egl/main/eglsync.c
> > +++ b/src/egl/main/eglsync.c
> > @@ -40,7 +40,8 @@
> >  static EGLint
> >  _eglParseSyncAttribList(_EGLSync *sync, const EGLint *attrib_list)
> >  {
> > -   EGLint i, err = EGL_SUCCESS;
> > +   EGLint err = EGL_SUCCESS;
> > +   unsigned int i;
> > 
> >     if (!attrib_list)
> >        return EGL_SUCCESS;
> > @@ -69,7 +70,8 @@ _eglParseSyncAttribList(_EGLSync *sync, const EGLint *attrib_list)
> >  static EGLint
> >  _eglParseSyncAttribList64(_EGLSync *sync, const EGLAttrib *attrib_list)
> >  {
> > -   EGLint i, err = EGL_SUCCESS;
> > +   EGLint err = EGL_SUCCESS;
> > +   unsigned int i;
> > 
> >     if (!attrib_list)
> >        return EGL_SUCCESS;
> >