Re: [Mesa-dev] glxinfo proposed change

[Ian Romanick]

On 01/07/2013 01:08 PM, Jorge Ramirez Ortiz, HCL Europe wrote:


-----Original Message-----
From: Ian Romanick [mailto:i...@freedesktop.org]
Sent: Monday, January 07, 2013 7:48 PM
To: Jorge Ramirez Ortiz, HCL Europe
Cc: Dan Nicholson; mesa-dev@lists.freedesktop.org
Subject: Re: [Mesa-dev] glxinfo proposed change

On 01/07/2013 06:35 AM, Jorge Ramirez Ortiz, HCL Europe wrote:
-----Original Message-----
From: Dan Nicholson [mailto:dbn.li...@gmail.com]
Sent: Monday, January 07, 2013 2:31 PM
To: Jorge Ramirez Ortiz, HCL Europe
Cc: mesa-dev@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-or...@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@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.

You are absolutely wrong. NULL is a valid return value. Please read the code.

I've been working on this project for almost twelve years, and I've been 
a voting member of the OpenGL Architecture Review Board for almost as 
long.  I'm quite familiar with the code.

glGetString returns 0 only when an error is generated, and it is not 
allowed to generate an error for a valid enum, such as GL_VERSION, when 
there is a context.  Previous code in glxinfo has already made the 
context current and checked that it was successful.

I wasn't interested in seeing this patch ever land, and after your rude, 
disrespectful attitude, I'm even less interested.

_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev