fixed 2 pthreads issues, added noXCursor option.

CHANGES

 0.2
+   added flag to optionally not send XCursor updates.
+   fixed java viewer on server side:
+	SendCursorUpdate would send data even before the client pixel format
+	was set.
+   fixed two pthread issues:
+	rfbSendFramebuffer was sent by a ProcessClientMessage function
+	(unprotected by updateMutex).
+	cursor coordinates were set without protection by cursorMutex
    source is now equivalent to TridiaVNC 1.2.1
-   pthreads now work (use the iterators!)
+   pthreads now work (use iterators!)
    cursors are supported (rfbSetCursor automatically undraws cursor)
    support for 3 bytes/pixel (slow!)
    server side colourmap support

TODO

@@ -5,9 +5,6 @@ udp
 documentation
 	perhaps the option (or just hint) not to mark very tiny regions as
 	modified, because that is inefficient for the encodings.
-optionally dont draw rich cursors as xcursors
-cursor smears on IRIX with pthreads, then has bus error. has to be a mutex
-	problem in cursor routines.
 
 later:
 ------
@@ -18,6 +15,9 @@ CORBA
 done:
 -----
 
+.optionally dont draw rich cursors as xcursors
+.cursor smears on IRIX with pthreads, then has bus error. has to be a mutex
+	problem in cursor routines.
 .fix bug in http (java) client with big endian server: byte swapping is broken
 	(was a cursorshape which was sent too soon; java vncviewer assumes
 	a rich cursor shape to be always 1 byte per pixel, however, framebuffer

cursor.c

@@ -337,10 +337,9 @@ void rfbUndrawCursor(rfbClientPtr cl)
    rfbCursorPtr c=s->cursor;
    int j,x1,x2,y1,y2,bpp=s->rfbServerFormat.bitsPerPixel/8,
      rowstride=s->paddedWidthInBytes;
-
-   LOCK(cl->screen->cursorMutex);
+   LOCK(s->cursorMutex);
    if(!s->cursorIsDrawn) {
-     UNLOCK(cl->screen->cursorMutex);
+     UNLOCK(s->cursorMutex);
      return;
    }
    
@@ -350,7 +349,7 @@ void rfbUndrawCursor(rfbClientPtr cl)
    if(x1<0) x1=0;
    if(x2>=s->width) x2=s->width-1;
    x2-=x1; if(x2<=0) {
-     UNLOCK(cl->screen->cursorMutex);
+     UNLOCK(s->cursorMutex);
      return;
    }
    y1=s->cursorY-c->yhot;
@@ -358,9 +357,11 @@ void rfbUndrawCursor(rfbClientPtr cl)
    if(y1<0) y1=0;
    if(y2>=s->height) y2=s->height-1;
    y2-=y1; if(y2<=0) {
-     UNLOCK(cl->screen->cursorMutex);
+     UNLOCK(s->cursorMutex);
      return;
    }
+
+   /* get saved data */
    for(j=0;j<y2;j++)
      memcpy(s->frameBuffer+(y1+j)*rowstride+x1*bpp,
 	    s->underCursorBuffer+j*x2*bpp,
@@ -368,7 +369,7 @@ void rfbUndrawCursor(rfbClientPtr cl)
    
    rfbMarkRectAsModified(s,x1,y1,x1+x2,y1+y2);
    s->cursorIsDrawn = FALSE;
-   UNLOCK(cl->screen->cursorMutex);
+   UNLOCK(s->cursorMutex);
 }
 
 void rfbDrawCursor(rfbClientPtr cl)
@@ -379,10 +380,10 @@ void rfbDrawCursor(rfbClientPtr cl)
      rowstride=s->paddedWidthInBytes,
      bufSize,w;
    if(!c) return;
-   LOCK(cl->screen->cursorMutex);
+   LOCK(s->cursorMutex);
    if(s->cursorIsDrawn) {
      /* is already drawn */
-     UNLOCK(cl->screen->cursorMutex);
+     UNLOCK(s->cursorMutex);
      return;
    }
    bufSize=c->width*c->height*bpp;
@@ -400,7 +401,7 @@ void rfbDrawCursor(rfbClientPtr cl)
    if(x1<0) { i1=-x1; x1=0; }
    if(x2>=s->width) x2=s->width-1;
    x2-=x1; if(x2<=0) {
-     UNLOCK(cl->screen->cursorMutex);
+     UNLOCK(s->cursorMutex);
      return; /* nothing to do */
    }
    y1=s->cursorY-c->yhot;
@@ -408,9 +409,11 @@ void rfbDrawCursor(rfbClientPtr cl)
    if(y1<0) { j1=-y1; y1=0; }
    if(y2>=s->height) y2=s->height-1;
    y2-=y1; if(y2<=0) {
-     UNLOCK(cl->screen->cursorMutex);
+     UNLOCK(s->cursorMutex);
      return; /* nothing to do */
    }
+
+   /* save data */
    for(j=0;j<y2;j++)
      memcpy(s->underCursorBuffer+j*x2*bpp,
 	    s->frameBuffer+(y1+j)*rowstride+x1*bpp,
@@ -426,9 +429,10 @@ void rfbDrawCursor(rfbClientPtr cl)
 	 memcpy(s->frameBuffer+(j+y1)*rowstride+(i+x1)*bpp,
 		c->richSource+(j+j1)*c->width*bpp+(i+i1)*bpp,bpp);
 
+
    rfbMarkRectAsModified(s,x1,y1,x1+x2,y1+y2);
    s->cursorIsDrawn = TRUE;
-   UNLOCK(cl->screen->cursorMutex);
+   UNLOCK(s->cursorMutex);
 }
 
 /* for debugging */

main.c

@@ -34,7 +34,7 @@
 #include "sraRegion.h"
 
 #ifdef HAVE_PTHREADS
-pthread_mutex_t logMutex;
+MUTEX(logMutex);
 #endif
 
 /*
@@ -48,7 +48,7 @@ rfbLog(char *format, ...)
     char buf[256];
     time_t log_clock;
 
-    IF_PTHREADS(LOCK(logMutex));
+    LOCK(logMutex);
     va_start(args, format);
 
     time(&log_clock);
@@ -59,7 +59,7 @@ rfbLog(char *format, ...)
     fflush(stderr);
 
     va_end(args);
-    IF_PTHREADS(UNLOCK(logMutex));
+    UNLOCK(logMutex);
 }
 
 void rfbLogPerror(char *str)
@@ -116,25 +116,28 @@ clientOutput(void *data)
 
     while (1) {
         haveUpdate = false;
-        LOCK(cl->updateMutex);
         while (!haveUpdate) {
             if (cl->sock == -1) {
                 /* Client has disconnected. */
-	        UNLOCK(cl->updateMutex);
                 return NULL;
             }
-	    updateRegion = sraRgnCreateRgn(cl->modifiedRegion);
-	    haveUpdate = sraRgnAnd(updateRegion,cl->requestedRegion);
-	    sraRgnDestroy(updateRegion);
+	    LOCK(cl->updateMutex);
+	    haveUpdate = FB_UPDATE_PENDING(cl);
+	    if(!haveUpdate) {
+		updateRegion = sraRgnCreateRgn(cl->modifiedRegion);
+		haveUpdate = sraRgnAnd(updateRegion,cl->requestedRegion);
+		sraRgnDestroy(updateRegion);
+	    }
+	    UNLOCK(cl->updateMutex);
 
             if (!haveUpdate) {
                 WAIT(cl->updateCond, cl->updateMutex);
+		UNLOCK(cl->updateMutex); /* we really needn't lock now. */
             }
         }
         
         /* OK, now, to save bandwidth, wait a little while for more
            updates to come along. */
-        UNLOCK(cl->updateMutex);
         usleep(rfbDeferUpdateTime * 1000);
 
         /* Now, get the region we're going to update, and remove
@@ -143,12 +146,10 @@ clientOutput(void *data)
            is updated, we'll be sure to do another update later. */
         LOCK(cl->updateMutex);
 	updateRegion = sraRgnCreateRgn(cl->modifiedRegion);
-	sraRgnAnd(updateRegion,cl->requestedRegion);
-	sraRgnSubtract(cl->modifiedRegion,updateRegion);
+        UNLOCK(cl->updateMutex);
 
         /* Now actually send the update. */
         rfbSendFramebufferUpdate(cl, updateRegion);
-        UNLOCK(cl->updateMutex);
 
 	sraRgnDestroy(updateRegion);
     }
@@ -264,13 +265,14 @@ void
 defaultPtrAddEvent(int buttonMask, int x, int y, rfbClientPtr cl)
 {
    if(x!=cl->screen->cursorX || y!=cl->screen->cursorY) {
-      Bool cursorWasDrawn=cl->screen->cursorIsDrawn;
-      if(cursorWasDrawn)
+      if(cl->screen->cursorIsDrawn)
 	rfbUndrawCursor(cl);
-      cl->screen->cursorX = x;
-      cl->screen->cursorY = y;
-      if(cursorWasDrawn)
-	rfbDrawCursor(cl);
+      LOCK(cl->screen->cursorMutex);
+      if(!cl->screen->cursorIsDrawn) {
+	  cl->screen->cursorX = x;
+	  cl->screen->cursorY = y;
+      }
+      UNLOCK(cl->screen->cursorMutex);
    }
 }
 
@@ -311,6 +313,8 @@ rfbScreenInfoPtr rfbGetScreen(int argc,char** argv,
    rfbScreenInfoPtr rfbScreen=malloc(sizeof(rfbScreenInfo));
    rfbPixelFormat* format=&rfbScreen->rfbServerFormat;
 
+   INIT_MUTEX(logMutex);
+
    if(width&3)
      fprintf(stderr,"WARNING: Width (%d) is not a multiple of 4. VncViewer has problems with that.\n",width);
 
@@ -387,7 +391,9 @@ rfbScreenInfoPtr rfbGetScreen(int argc,char** argv,
    rfbScreen->dontSendFramebufferUpdate = FALSE;
    rfbScreen->cursorX=rfbScreen->cursorY=rfbScreen->underCursorBufferLen=0;
    rfbScreen->underCursorBuffer=NULL;
-   //INIT_MUTEX(rfbScreen->cursorMutex);
+   rfbScreen->dontConvertRichCursorToXCursor = FALSE;
+   rfbScreen->cursor = &myCursor;
+   INIT_MUTEX(rfbScreen->cursorMutex);
 
    /* proc's and hook's */
 
@@ -396,7 +402,6 @@ rfbScreenInfoPtr rfbGetScreen(int argc,char** argv,
    rfbScreen->ptrAddEvent = defaultPtrAddEvent;
    rfbScreen->setXCutText = defaultSetXCutText;
    rfbScreen->getCursorPtr = defaultGetCursorPtr;
-   rfbScreen->cursor = &myCursor;
    rfbScreen->setTranslateFunction = rfbSetTranslateFunction;
    rfbScreen->newClientHook = doNothingWithClient;
 
@@ -421,7 +426,6 @@ void rfbInitServer(rfbScreenInfoPtr rfbScreen)
 {
   rfbInitSockets(rfbScreen);
   httpInitSockets(rfbScreen);
-  INIT_MUTEX(logMutex);
 }
 
 void
@@ -448,8 +452,6 @@ rfbProcessEvents(rfbScreenInfoPtr rfbScreen,long usec)
 
 void rfbRunEventLoop(rfbScreenInfoPtr rfbScreen, long usec, Bool runInBackground)
 {
-  rfbInitServer(rfbScreen);
-
   if(runInBackground) {
 #ifdef HAVE_PTHREADS
        pthread_t listener_thread;

rfb.h

@@ -78,20 +78,20 @@ int max(int,int);
 #ifdef HAVE_PTHREADS
 #include <pthread.h>
 #if 0
-#define LOCK(mutex) fprintf(stderr,"%s:%d LOCK(%s)\n",__FILE__,__LINE__,#mutex)
-#define UNLOCK(mutex) fprintf(stderr,"%s:%d UNLOCK(%s)\n",__FILE__,__LINE__,#mutex)
-#define MUTEX(mutex)
-#define INIT_MUTEX(mutex) fprintf(stderr,"%s:%d INIT_MUTEX(%s)\n",__FILE__,__LINE__,#mutex)
+#define LOCK(mutex) fprintf(stderr,"%s:%d LOCK(%s,0x%x)\n",__FILE__,__LINE__,#mutex,&(mutex))
+#define UNLOCK(mutex) fprintf(stderr,"%s:%d UNLOCK(%s,0x%x)\n",__FILE__,__LINE__,#mutex,&(mutex))
+#define MUTEX(mutex) int mutex
+#define INIT_MUTEX(mutex) fprintf(stderr,"%s:%d INIT_MUTEX(%s,0x%x)\n",__FILE__,__LINE__,#mutex,&(mutex))
 #define TINI_MUTEX(mutex) fprintf(stderr,"%s:%d TINI_MUTEX(%s)\n",__FILE__,__LINE__,#mutex)
 #define SIGNAL(cond) fprintf(stderr,"%s:%d SIGNAL(%s)\n",__FILE__,__LINE__,#cond)
-#define WAIT(cond,mutex) fprintf(stderr,"%s:%d WAIT(%s,%s)\n",__FILE__,__LINE__,#cond,#mutex)
+#define WAIT(cond,mutex) /* fprintf(stderr,"%s:%d WAIT(%s,%s)\n",__FILE__,__LINE__,#cond,#mutex) */
 #define COND(cond)
 #define INIT_COND(cond) fprintf(stderr,"%s:%d INIT_COND(%s)\n",__FILE__,__LINE__,#cond)
 #define TINI_COND(cond) fprintf(stderr,"%s:%d TINI_COND(%s)\n",__FILE__,__LINE__,#cond)
-#define IF_PTHREAD(x)
+#define IF_PTHREADS(x)
 #else
-#define LOCK(mutex) pthread_mutex_lock(&(mutex))
-#define UNLOCK(mutex) pthread_mutex_unlock(&(mutex))
+#define LOCK(mutex) pthread_mutex_lock(&(mutex));
+#define UNLOCK(mutex) pthread_mutex_unlock(&(mutex));
 #define MUTEX(mutex) pthread_mutex_t (mutex)
 #define INIT_MUTEX(mutex) pthread_mutex_init(&(mutex),NULL)
 #define TINI_MUTEX(mutex) pthread_mutex_destroy(&(mutex))
@@ -203,10 +203,6 @@ typedef struct
     Bool dontSendFramebufferUpdate; /* TRUE while removing or drawing the
 				       cursor */
    
-    /* these variables are needed to save the area under the cursor */
-    int cursorX, cursorY,underCursorBufferLen;
-    char* underCursorBuffer;
-   
     /* additions by libvncserver */
 
     rfbPixelFormat rfbServerFormat;
@@ -242,6 +238,11 @@ typedef struct
     Bool rfbNeverShared;
     Bool rfbDontDisconnect;
     struct rfbClientRec* rfbClientHead;
+
+    /* cursor */
+    int cursorX, cursorY,underCursorBufferLen;
+    char* underCursorBuffer;
+    Bool dontConvertRichCursorToXCursor;
     struct rfbCursor* cursor;
     MUTEX(cursorMutex);
    
@@ -254,8 +255,6 @@ typedef struct
     GetCursorProcPtr getCursorPtr;
     SetTranslateFunctionProcPtr setTranslateFunction;
   
-    /* the following members are hooks, i.e. they are called if set,
-       but not overriding original functionality */
     /* newClientHook is called just after a new client is created */
     NewClientHookPtr newClientHook;
 

rfbserver.c

@@ -73,6 +73,7 @@ void
 rfbClientListInit(rfbScreenInfoPtr rfbScreen)
 {
     rfbScreen->rfbClientHead = NULL;
+    INIT_MUTEX(rfbClientListMutex);
 }
 
 rfbClientIteratorPtr
@@ -657,10 +658,12 @@ rfbProcessClientNormalMessage(cl)
 		}
 		break;
 	    case rfbEncodingXCursor:
-		rfbLog("Enabling X-style cursor updates for client %s\n",
-		       cl->host);
-		cl->enableCursorShapeUpdates = TRUE;
-		cl->cursorWasChanged = TRUE;
+		if(!cl->screen->dontConvertRichCursorToXCursor) {
+		    rfbLog("Enabling X-style cursor updates for client %s\n",
+			   cl->host);
+		    cl->enableCursorShapeUpdates = TRUE;
+		    cl->cursorWasChanged = TRUE;
+		}
 		break;
 	    case rfbEncodingRichCursor:
 	        rfbLog("Enabling full-color cursor updates for client "
@@ -729,6 +732,7 @@ rfbProcessClientNormalMessage(cl)
 	    if (!cl->format.trueColour) {
 		if (!rfbSetClientColourMap(cl, 0, 0)) {
 		    sraRgnDestroy(tmpRegion);
+		    UNLOCK(cl->updateMutex);
 		    return;
 		}
 	    }
@@ -740,10 +744,7 @@ rfbProcessClientNormalMessage(cl)
        }
        SIGNAL(cl->updateCond);
        UNLOCK(cl->updateMutex);
-	
-       if(cl->sock>=0 && FB_UPDATE_PENDING(cl)) {
-	 rfbSendFramebufferUpdate(cl,cl->modifiedRegion);
-       }
+
        sraRgnDestroy(tmpRegion);
 
        return;
@@ -847,16 +848,12 @@ rfbSendFramebufferUpdate(cl, givenUpdateRegion)
     sraRegionPtr updateRegion,updateCopyRegion;
     int dx, dy;
     Bool sendCursorShape = FALSE;
-    Bool cursorWasDrawn = FALSE;
-
     
     /*
      * If this client understands cursor shape updates, cursor should be
      * removed from the framebuffer. Otherwise, make sure it's put up.
      */
 
-    cursorWasDrawn = cl->screen->cursorIsDrawn;
-
     if (cl->enableCursorShapeUpdates) {
       if (cl->screen->cursorIsDrawn) {
 	rfbUndrawCursor(cl);
@@ -869,7 +866,9 @@ rfbSendFramebufferUpdate(cl, givenUpdateRegion)
 	rfbDrawCursor(cl);
       }
     }
-   
+
+    LOCK(cl->updateMutex);
+
     /*
      * The modifiedRegion may overlap the destination copyRegion.  We remove
      * any overlapping bits from the copyRegion (since they'd only be
@@ -889,6 +888,7 @@ rfbSendFramebufferUpdate(cl, givenUpdateRegion)
     sraRgnOr(updateRegion,cl->copyRegion);
     if(!sraRgnAnd(updateRegion,cl->requestedRegion) && !sendCursorShape) {
       sraRgnDestroy(updateRegion);
+      UNLOCK(cl->updateMutex);
       return TRUE;
     }
 
@@ -927,12 +927,14 @@ rfbSendFramebufferUpdate(cl, givenUpdateRegion)
      sraRgnOr(cl->modifiedRegion,cl->copyRegion);
      sraRgnSubtract(cl->modifiedRegion,updateRegion);
      sraRgnSubtract(cl->modifiedRegion,updateCopyRegion);
-   
+
      sraRgnMakeEmpty(cl->requestedRegion);
      sraRgnMakeEmpty(cl->copyRegion);
      cl->copyDX = 0;
      cl->copyDY = 0;
    
+     UNLOCK(cl->updateMutex);
+   
    /*
      * Now send the update.
      */
@@ -1066,13 +1068,6 @@ rfbSendFramebufferUpdate(cl, givenUpdateRegion)
         return FALSE;
     }
 
-    if(cursorWasDrawn != cl->screen->cursorIsDrawn) {
-       if(cursorWasDrawn)
-	  rfbDrawCursor(cl);
-       else
-	  rfbUndrawCursor(cl);
-    }
-
     sraRgnDestroy(updateRegion);
     return TRUE;
 }