Egbert Eich wrote:
There is a report in bugzilla (#439) which claims:
the bug is in xc/lib/GL/glx/glxcmds.c int bufSize = XMaxRequestSize(dpy) * 4;
should be int bufSize = XMaxRequestSize(dpy) * 4 - 8;
or more cleanly
int bufSize = XMaxRequestSize(dpy) * 4 - sizeof(xGLXRenderReq);
it happens that you may completely fill your GLX buffer if you use variable size command larger than 156 bytes (and smaller than 4096 bytes)
in that case you find yourself with an X command larger than 256Kbytes. This
is very unlikely but possible. It explain why this bug has not shown itself
before in this very old SGI code.
I've briefly looked at the code and it seems to be correct. However I would like to double check before I commit anything.
Any opinions?
I'm not sure this is correct. bufSize is used to allocate the buffer (gc->buf in the code) that will hold the commands, including the xGLXRenderReq header. I've been doing a lot of work lately on the GLX code (both client-side & server-side) in the DRI tree lately. I'll take a look at this a bit more and get back to you.
I looked into the code, and I now understand what's going on. Alexis made a good catch of a very subtle bug! The main problem that I had was that it wasn't 100% clear at first glance how bufSize / buf / pc were used. Some form of "- 8" should be applied to bufSize. I have attached the patch that I plan to apply to the DRI tree. I suspect that it has only cosmetic and / or commentary differences from your patch.
Some things have moved around in the DRI tree, so this patch probably won't apply to the XFree86 tree.
Index: glxcmds.c
===================================================================
RCS file: /cvsroot/dri/xc/xc/lib/GL/glx/glxcmds.c,v
retrieving revision 1.44
diff -u -d -r1.44 glxcmds.c
--- glxcmds.c 25 Jun 2003 00:39:58 -0000 1.44
+++ glxcmds.c 30 Jun 2003 20:49:15 -0000
@@ -198,7 +261,7 @@
GLXContext AllocateGLXContext( Display *dpy )
{
GLXContext gc;
- int bufSize = XMaxRequestSize(dpy) * 4;
+ int bufSize;
CARD8 opcode;
if (!dpy)
@@ -217,7 +280,14 @@
}
memset(gc, 0, sizeof(struct __GLXcontextRec));
- /* Allocate transport buffer */
+ /*
+ ** Create a temporary buffer to hold GLX rendering commands. The size
+ ** of the buffer is selected so that the maximum number of GLX rendering
+ ** commands can fit in a single X packet and still have room in the X
+ ** packed to for the GLXRenderReq header.
+ */
+
+ bufSize = (XMaxRequestSize(dpy) * 4) - sz_xGLXRenderReq;
gc->buf = (GLubyte *) Xmalloc(bufSize);
if (!gc->buf) {
Xfree(gc);
