[Mesa-dev] glxinfo proposed change

[Brian Paul]

On 01/07/2013 11:48 AM, Ian Romanick wrote:
> On 01/07/2013 06:35 AM, Jorge Ramirez Ortiz, HCL Europe wrote:
>>> -----Original Message-----
>>> From: Dan Nicholson [mailto:dbn.lists at gmail.com]
>>> Sent: Monday, January 07, 2013 2:31 PM
>>> To: Jorge Ramirez Ortiz, HCL Europe
>>> Cc: mesa-dev at lists.freedesktop.org
>>> Subject: Re: [Mesa-dev] glxinfo proposed change
>>>
>>> On Mon, Jan 7, 2013 at 5:06 AM, Jorge Ramirez Ortiz, HCL Europe
>>> <Jorge.Ramirez-Ortiz at hcl.com> wrote:
>>>> Hi all,
>>>>
>>>> Does this patch make sense? glVersion can be NULL and not having the
>>> changes below could cause a SIGSEGV
>>>>
>>>> [jramirez at calypso-2 mesa-demos.git (tmp *)]$ git diff
>>>> src/xdemos/glxinfo.c
>>>> diff --git a/src/xdemos/glxinfo.c b/src/xdemos/glxinfo.c
>>>> index aa6430d..97b6539 100644
>>>> --- a/src/xdemos/glxinfo.c
>>>> +++ b/src/xdemos/glxinfo.c
>>>> @@ -639,7 +639,7 @@ print_screen_info(Display *dpy, int scrnum, Bool
>>> allowDirect, Bool limits, Bool
>>>> printf("OpenGL renderer string: %s\n", glRenderer);
>>>> printf("OpenGL version string: %s\n", glVersion);
>>>> #ifdef GL_VERSION_2_0
>>>> - if (glVersion[0] >= '2' && glVersion[1] == '.') {
>>>> + if (glVersion && glVersion[0] >= '2' && glVersion[1] == '.') {
>>>> char *v = (char *) glGetString(GL_SHADING_LANGUAGE_VERSION);
>>>> printf("OpenGL shading language version string: %s\n", v);
>>>> }
>>>
>>> Normally, this is a good idea. However, glVersion is already
>>> dereferenced in the printf on the previous line. So, any actual
>>> checking for NULL glVersion should at least come earlier in the
>>> function. On the other hand, returning NULL from
>>> glGetString(GL_VERSION) might be so rare that bothering to guard for
>>> it would be more effort than it's worth.
>>>
>>
>> Bear in mind that printf of a null string wont SIGSEGV.
>> Only trying to access glVersion[] elements presents -and indeed is -
>> a risk and therefore a critical bug (for my usual standards that is).
>
> The critical bug is that glGetString is not allowed to return NULL.
> Your GL is horrifically broken in that case, and no application will
> work either.

glGetString() will return NULL if there's no bound context.  But if 
glXMakeCurrent() fails, glxinfo won't be calling glGetString().

Jorge, have you actually seen glxinfo fail with glVersion==NULL or is 
this just something you spotted in the code?

-Brian