From e1288f7e1bda85c619573c01fe9fdb84aa6e10b4 Mon Sep 17 00:00:00 2001 From: Stephen Heumann Date: Sat, 28 May 2016 21:59:35 -0500 Subject: [PATCH] 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 | 5 +---- copyrect.cc | 4 ---- desktopsize.cc | 2 -- hextile.cc | 5 +---- mouse.cc | 4 ---- raw.cc | 8 +------- vncdisplay.cc | 12 ++++-------- vncsession.cc | 33 ++++++--------------------------- vncsession.h | 7 +++++++ 9 files changed, 20 insertions(+), 60 deletions(-) diff --git a/clipboard.cc b/clipboard.cc index 2f6bc20..99f7a9a 100644 --- a/clipboard.cc +++ b/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 to be fixed */ PutScrap(textLen, textScrap, (Pointer) *readBufferHndl); /* Potential errors (e.g. out of memory) ignored */ - HUnlock(readBufferHndl); + DoneWithReadBuffer(); } } diff --git a/copyrect.cc b/copyrect.cc index 60cd065..a92002b 100644 --- a/copyrect.cc +++ b/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 */ } diff --git a/desktopsize.cc b/desktopsize.cc index 5aeaaea..b6c041d 100644 --- a/desktopsize.cc +++ b/desktopsize.cc @@ -81,8 +81,6 @@ void DoDesktopSize (void) { SetDataSize(fbWidth, fbHeight, vncWindow); DrawControls(vncWindow); - displayInProgress = FALSE; - NextRect(); /* Prepare for next rect */ } diff --git a/hextile.cc b/hextile.cc index 98c6ca4..8535c9d 100644 --- a/hextile.cc +++ b/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); } } diff --git a/mouse.cc b/mouse.cc index 997f862..ccfe756 100644 --- a/mouse.cc +++ b/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 */ } diff --git a/raw.cc b/raw.cc index 8e81096..41df6d7 100644 --- a/raw.cc +++ b/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 */ } diff --git a/vncdisplay.cc b/vncdisplay.cc index ae8103b..f26447b 100644 --- a/vncdisplay.cc +++ b/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; diff --git a/vncsession.cc b/vncsession.cc index 1e27922..9c46eb1 100644 --- a/vncsession.cc +++ b/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); - if (! DoWaitingReadTCP(4)) + case vncConnectionFailed: 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); - AlertWindow(awResource, NULL, + default: 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; diff --git a/vncsession.h b/vncsession.h index 02850bf..2b9a890 100644 --- a/vncsession.h +++ b/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);