Re: winex11: Don't trace a garbage value or read past end of caller's array in X11DRV_wglChoosePixelFormatARB().

[Roderick Colenbrander]

That change looks suspicious. If switching to i+1 fixes a problem, the real
problem is that the if-statement in question gets entered in the first
place. Some bounds checking on the outer for-loop (the one which loops
through nCfgs and nMaxFormats), is probably wrong then. I had issues with
this part in the past, so there maybe a small typo (it may just be off by
1).

Roderick


On Thu, Sep 19, 2013 at 6:37 PM, Ken Thomases <ken(a)codeweavers.com> wrote:
> ---
> dlls/winex11.drv/opengl.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)

winex11: Don't trace a garbage value or read past end of caller's array in X11DRV_wglChoosePixelFormatARB().

[Ken Thomases]

On Sep 20, 2013, at 9:44 AM, Roderick Colenbrander wrote:
> That change looks suspicious. If switching to i+1 fixes a problem, the real
> problem is that the if-statement in question gets entered in the first
> place. Some bounds checking on the outer for-loop (the one which loops
> through nCfgs and nMaxFormats), is probably wrong then. I had issues with
> this part in the past, so there maybe a small typo (it may just be off by
> 1).

This has already been committed, but I'll clarify.  The old code looked like:

                    piFormats[pfmt_it++] = i + 1;
                    TRACE("at %d/%d found FBCONFIG_ID 0x%x (%d)\n",
                          it + 1, nCfgs, fmt_id, piFormats[pfmt_it]);

The problem is that pfmt_it was incremented in the first statement so it is no longer correct in the TRACE.  It now refers to the array element after the one that was assigned.  That may read past the end of piFormats.  In any case, it will read an element which has not yet been assigned.

I could have moved the increment to after both statements.  Or I could have used "pfmt_it - 1" in the TRACE.  It seemed simplest to just use the value that we know is the found pixel format.

-Ken