Small refactoring to reduce boilerplate code

DoReadTCP automatically disposes the old read buffer handle before doing the read, and then locks the new read buffer handle before returning. This removes the need for most explicit locking/unlocking of the handle. A macro is provided to dispose of the handle earlier (useful in a few cases where it may be big, and another read isn't immediately performed).

Also, set displayInProgress to FALSE in NextRect, rather than in code for each encoding.

clipboard.cc

@@ -48,9 +48,7 @@ void DoServerCutText (void) {
         DoClose(vncWindow);
         return;
     }
-    HLock(readBufferHndl);
     textLen = SwapBytes4((unsigned long) **readBufferHndl);
-    HUnlock(readBufferHndl);
 
     if (! DoWaitingReadTCP(textLen)) {
         DoClose(vncWindow);
@@ -58,7 +56,6 @@ void DoServerCutText (void) {
     };
     if (allowClipboardTransfers) {
         ZeroScrap();
-        HLock(readBufferHndl);
                     
         /* Convert lf->cr; Use pointer arithmetic so we can go over 64k */
         for (i = 0; i < textLen; i++)
@@ -68,7 +65,7 @@ void DoServerCutText (void) {
                     /* Below function call requires <scrap.h> to be fixed */
         PutScrap(textLen, textScrap, (Pointer) *readBufferHndl);
         /* Potential errors (e.g. out of memory) ignored */
-        HUnlock(readBufferHndl);
+        DoneWithReadBuffer();
     }
 }
     

copyrect.cc

@@ -48,11 +48,9 @@ void DoCopyRect (void) {
 
     contentOrigin = GetContentOrigin(vncWindow);
 
-    HLock(readBufferHndl);
     dataPtr = (unsigned int *) ((char *) (*readBufferHndl));
     srcRect.h1 = SwapBytes2(dataPtr[0]) - contentOriginPtr->h;
     srcRect.v1 = SwapBytes2(dataPtr[1]) - contentOriginPtr->v;
-    HUnlock(readBufferHndl);
 
     srcRect.h2 = srcRect.h1 + rectWidth;
     srcRect.v2 = srcRect.v1 + rectHeight;
@@ -72,8 +70,6 @@ void DoCopyRect (void) {
             rectX - contentOriginPtr->h, rectY - contentOriginPtr->v, modeCopy);
 
 done:
-    displayInProgress = FALSE;
-
     NextRect();                                     /* Prepare for next rect */
 }
 

desktopsize.cc

@@ -81,8 +81,6 @@ void DoDesktopSize (void) {
     SetDataSize(fbWidth, fbHeight, vncWindow);
     DrawControls(vncWindow);
 
-    displayInProgress = FALSE;
-
     NextRect();             /* Prepare for next rect */
 }
 

hextile.cc

@@ -57,7 +57,6 @@ static void HexNextTile (void) {
     if (hexXTileNum == hexXTiles) {
         hexYTileNum++;
         if (hexYTileNum == hexYTiles) {     /* Done with this Hextile rect */
-            displayInProgress = FALSE;
             NextRect();
             return;
         }                      
@@ -172,7 +171,7 @@ static void HexRawDraw (Point *contentOriginPtr, int rectWidth, int rectHeight)
 /* The macros below are used in HexDispatch() */
 #define HexDispatch_NextTile()  do {                                \
     HexNextTile();                                                  \
-    HUnlock(readBufferHndl);                                        \
+    DoneWithReadBuffer();                                           \
     /* Set up for next time */                                      \
     status = hexWaitingForSubencoding;                              \
     bytesNeeded = 1;                                                \
@@ -211,7 +210,6 @@ void HexDispatch (void) {
 
     /* If we don't have the next bit of needed data yet, return. */
     while (DoReadTCP(bytesNeeded)) {
-        HLock(readBufferHndl);
         dataPtr = *(unsigned char **) readBufferHndl;
         /* If we're here, readBufferHndl contains bytesNeeded bytes of data. */
         switch (status) {
@@ -280,7 +278,6 @@ void HexDispatch (void) {
                 HexDispatch_NextTile();
             }
         }
-            HUnlock(readBufferHndl);
     }
 }
 

mouse.cc

@@ -162,8 +162,6 @@ void DoCursor (void) {
     if (!DoReadTCP((unsigned long)rectWidth * rectHeight + bitmaskLineBytes * rectHeight))
         return; /* Try again later */
 
-    HLock(readBufferHndl);
-
     cursorPixels = (unsigned char *)(*readBufferHndl);
     bitmask = (unsigned char *)(*readBufferHndl) + (unsigned long)rectWidth * rectHeight;
 
@@ -316,8 +314,6 @@ void DoCursor (void) {
 #endif
 
 done:
-    HUnlock(readBufferHndl);
     free(oldCursor);
-    displayInProgress = FALSE;
     NextRect();             /* Prepare for next rect */
 }

raw.cc

@@ -57,10 +57,6 @@ unsigned char * rawDecode320(unsigned startOffset, unsigned endOffset,
  * because the rectangle is not visible.
  */
 static void StopRawDrawing (void) {
-    HUnlock(readBufferHndl);
-
-    displayInProgress = FALSE;
-
     NextRect();                                     /* Prepare for next rect */
 }
 
@@ -237,7 +233,6 @@ static void RawDrawLine (void) {
     srcRect.v1 = 0;
     srcRect.v2 = 1;
 
-    HLock(readBufferHndl);
     dataPtr = (unsigned char *) *readBufferHndl;
     SetPort(vncWindow);                         /* Drawing in VNC window */
 
@@ -266,7 +261,7 @@ static void RawDrawLine (void) {
             }
         }
 
-    HUnlock(readBufferHndl);
+    DoneWithReadBuffer();
     contentOrigin = GetContentOrigin(vncWindow);
     PPToPort(&srcLocInfo, &srcRect, rectX - contentOriginPtr->h,
             rectY - contentOriginPtr->v, modeCopy);
@@ -359,5 +354,4 @@ void DoRawRect (void) {
     displayInProgress = TRUE;
     drawingLine = 0;            /* Drawing first line of rect */
     checkBounds = TRUE;         /* Flag to check bounds when drawing 1st line */
-    HLock(readBufferHndl);      /* Lock handle just once for efficiency */
 }

vncdisplay.cc

@@ -226,7 +226,6 @@ static void DoFBUpdate (void) {
         //printf("Closing in DoFBUpdate\n");
         return;
     }
-    HLock(readBufferHndl);
     dataPtr = (unsigned int *) (((char *) (*readBufferHndl)) + 1);
     numRects = SwapBytes2(dataPtr[0]);                  /* Get data */
     rectX = SwapBytes2(dataPtr[1]);
@@ -234,7 +233,6 @@ static void DoFBUpdate (void) {
     rectWidth  = SwapBytes2(dataPtr[3]);
     rectHeight = SwapBytes2(dataPtr[4]);
     rectEncoding = SwapBytes4(*(unsigned long *)(dataPtr + 5));
-    HUnlock(readBufferHndl);
 }
 
 /* The server should never send a color map, since we don't use a mapped
@@ -249,9 +247,7 @@ static void DoSetColourMapEntries (void) {
 
     DoWaitingReadTCP(3);
     DoWaitingReadTCP(2);
-    HLock(readBufferHndl);
     numColors = SwapBytes2((unsigned int) **readBufferHndl);
-    HUnlock(readBufferHndl);
     for (; numColors > 0; numColors--) {
         DoWaitingReadTCP(6);
     }
@@ -265,6 +261,8 @@ static void DoSetColourMapEntries (void) {
 void NextRect (void) {
     unsigned int *dataPtr;
 
+    displayInProgress = FALSE;
+    
     numRects--;
     if (numRects) {                 /* Process next rectangle */
         if (!DoWaitingReadTCP(12)) {
@@ -272,20 +270,20 @@ void NextRect (void) {
             DoClose(vncWindow);
             return;
         }
-        HLock(readBufferHndl);
         dataPtr = (unsigned int *) ((char *) (*readBufferHndl));
         rectX = SwapBytes2(dataPtr[0]);
         rectY = SwapBytes2(dataPtr[1]);
         rectWidth  = SwapBytes2(dataPtr[2]);
         rectHeight = SwapBytes2(dataPtr[3]);
         rectEncoding = SwapBytes4(*(unsigned long *)(dataPtr + 4));
-        HUnlock(readBufferHndl);
         //printf("New Rect: X = %u, Y = %u, Width = %u, Height = %u\n", rectX, rectY, rectWidth, rectHeight);
     }
     else {                          /* No more rectangles from last update */
         unsigned long contentOrigin;
         Point * contentOriginPtr = (void *) &contentOrigin;
 
+        DoneWithReadBuffer();
+
         contentOrigin = GetContentOrigin(vncWindow);
         SendFBUpdateRequest(TRUE, contentOriginPtr->h, contentOriginPtr->v,
                             winWidth, winHeight);
@@ -339,9 +337,7 @@ void ConnectedEventLoop (void) {
         }
     }
     else if (DoReadTCP(1)) {            /* Read message type byte */
-        HLock(readBufferHndl);
         messageType = ((unsigned char) **readBufferHndl);
-        HUnlock(readBufferHndl);
         switch (messageType) {
             case FBUpdate:              DoFBUpdate();
                                         break;

vncsession.cc

@@ -108,7 +108,6 @@ void DoConnect (void) {
     }
 
     if (DoVNCHandshaking() == FALSE) {
-        SetHandleSize(1,readBufferHndl);
         CloseConnectStatusWindow();
         InitCursor();
         SysBeep();
@@ -119,7 +118,6 @@ void DoConnect (void) {
         return;
     }
     if (FinishVNCHandshaking() == FALSE) {
-        SetHandleSize(1,readBufferHndl);
         CloseConnectStatusWindow();
         InitCursor();
         AlertWindow(awResource, NULL, badOptionNegotiationError);
@@ -320,6 +318,7 @@ static BOOLEAN ReadFixup (unsigned long requested, unsigned long returned) {
     SetHandleSize(requested, readBufferHndl);
     if (toolerror())
         return FALSE;
+    HLock(readBufferHndl);
 
     do {
         TCPIPPoll();
@@ -350,6 +349,7 @@ BOOLEAN DoReadTCP (unsigned long dataLength) {
     static srBuff theSRBuff;
     static rrBuff theRRBuff;
 
+    DoneWithReadBuffer();
     TCPIPPoll();
 
     if ((tcperr = TCPIPStatusTCP(hostIpid, &theSRBuff)) != tcperrOK)
@@ -360,7 +360,6 @@ BOOLEAN DoReadTCP (unsigned long dataLength) {
     if (theSRBuff.srRcvQueued < dataLength)
         return FALSE;
 
-    DisposeHandle(readBufferHndl);
     if ((tcperr = TCPIPReadTCP(hostIpid, buffTypeNewHandle, NULL,
                         dataLength, &theRRBuff)) != tcperrOK)
         return FALSE;
@@ -371,6 +370,7 @@ BOOLEAN DoReadTCP (unsigned long dataLength) {
     if (theRRBuff.rrBuffCount != dataLength)
         return ReadFixup(dataLength, theRRBuff.rrBuffCount);
 
+    HLock(readBufferHndl);
     return TRUE;
 }
 
@@ -393,18 +393,15 @@ static BOOLEAN DoVNCHandshaking (void) {
     strcpy(versionString, "");
     if (! DoWaitingReadTCP(12))
         return FALSE;
-    HLock(readBufferHndl);
     if ( ! ((strncmp((char *)*readBufferHndl, "RFB ", 4) == 0) &&
             (strncmp((char *)*readBufferHndl+4, RFBMAJORVERSIONSTR, 3) >= 0) &&
             (strncmp((char *)*readBufferHndl+7, ".", 1) == 0) &&
             (strncmp((char *)*readBufferHndl+11, "\n", 1) == 0))) {
-        HUnlock(readBufferHndl);
         InitCursor();
         AlertWindow(awResource, NULL, badRFBVersionAlert);
         alerted = TRUE;
         return FALSE;
     }
-    HUnlock(readBufferHndl);
 
     strcpy(versionString, RFBVERSIONSTR);
     if (TCPIPWriteTCP(hostIpid, versionString, 12, TRUE, FALSE)) {
@@ -417,20 +414,17 @@ static BOOLEAN DoVNCHandshaking (void) {
     if (! DoWaitingReadTCP(4)) {                /* Read authentication type */
         return FALSE;
     }
-    HLock(readBufferHndl);
     switch ((unsigned long) (**readBufferHndl)) {
-        case vncConnectionFailed:   HUnlock(readBufferHndl);
+        case vncConnectionFailed:   if (! DoWaitingReadTCP(4))
-                                    if (! DoWaitingReadTCP(4))
                                         return FALSE; 
                                     if (toolerror())
                                         return FALSE;
-                                    HLock(readBufferHndl);
                                     reasonLength = SwapBytes4(**readBufferHndl);
-                                    HUnlock(readBufferHndl);
                                     if (! DoWaitingReadTCP(reasonLength))
                                         return FALSE;
                                     if (toolerror())
                                         return FALSE;  
+                                    HUnlock(readBufferHndl);
                                     SetHandleSize(
                                             GetHandleSize(readBufferHndl)+1,
                                             readBufferHndl);
@@ -443,22 +437,18 @@ static BOOLEAN DoVNCHandshaking (void) {
                                                 (Pointer) readBufferHndl,
                                                 connectionFailedAlert);
                                         alerted = TRUE;
-                                        HUnlock(readBufferHndl);
                                     }
                                     return FALSE;
         case vncNoAuthentication:   break;
         case vncVNCAuthentication:  if (DoDES())
                                         break;
-                                    HUnlock(readBufferHndl);
                                     return FALSE;
-        default:                    HUnlock(readBufferHndl);
+        default:                    AlertWindow(awResource, NULL,
-                                    AlertWindow(awResource, NULL,
                                             badAuthTypeAlert);
                                     alerted = TRUE;
                                     return FALSE;
     }
 
-    HUnlock(readBufferHndl);
     return TRUE;
     #undef connectionFailedAlert
     #undef badRFBVersionAlert
@@ -547,10 +537,8 @@ static BOOLEAN DoDES (void) {
 
     DESAddParity(theKey, &vncPassword[1]);
 
-    HLock(readBufferHndl);
     DESCipher(theResponse, theKey, *(char **)readBufferHndl, modeEncrypt);
     DESCipher(&theResponse[8], theKey, *(char **)readBufferHndl+8, modeEncrypt);
-    HUnlock(readBufferHndl);
 
     if (TCPIPWriteTCP(hostIpid, theResponse, sizeof(theResponse), TRUE, FALSE))
         {
@@ -566,7 +554,6 @@ static BOOLEAN DoDES (void) {
         goto UnloadCrypto;
     }
 
-    HLock(readBufferHndl);
     if ((**readBufferHndl) == statusOK) {
         success = TRUE;
         goto UnloadCrypto;
@@ -590,8 +577,6 @@ static BOOLEAN DoDES (void) {
 
     UnloadCrypto:
 
-    HUnlock(readBufferHndl);
-
     if (startedCrypto) {
         CryptoShutDown();                       /* Shut down Crypto tool set */
         DisposeHandle(dpSpace);
@@ -678,14 +663,10 @@ static BOOLEAN FinishVNCHandshaking (void) {
     /* ServerInitialisation */
     if (! DoWaitingReadTCP(2))
         return FALSE;
-    HLock(readBufferHndl);
     fbWidth = SwapBytes2(**(unsigned **)readBufferHndl);
-    HUnlock(readBufferHndl);
     if (! DoWaitingReadTCP(2))
         return FALSE;
-    HLock(readBufferHndl);
     fbHeight = SwapBytes2(**(unsigned **)readBufferHndl);
-    HUnlock(readBufferHndl);
 
     if ((fbWidth > 16384) || (fbHeight > 16384)) {
         AlertWindow(awResource, NULL, screenTooBigError);
@@ -697,9 +678,7 @@ static BOOLEAN FinishVNCHandshaking (void) {
         return FALSE;
     if (! DoWaitingReadTCP(4))
         return FALSE;
-    HLock(readBufferHndl);
     serverNameLen = SwapBytes4(**(unsigned long **)readBufferHndl);
-    HUnlock(readBufferHndl);
     if (! DoWaitingReadTCP(serverNameLen))
         return FALSE;
 

vncsession.h

@@ -23,6 +23,13 @@ extern unsigned int hostIpid;
 extern void DisplayConnectStatus(char *, BOOLEAN);
 
 extern void DoConnect (void);
+
+#define DoneWithReadBuffer() do         \
+    if (readBufferHndl) {               \
+        DisposeHandle(readBufferHndl);  \
+        readBufferHndl = NULL;          \
+    } while (0)                         \
+
 extern BOOLEAN DoReadTCP (unsigned long);
 extern BOOLEAN DoWaitingReadTCP(unsigned long);
 extern void CloseTCPConnection (void);