--- a/ui/downloader_win.cpp	Thu Feb 20 15:44:09 2014 +0000
+++ b/ui/downloader_win.cpp	Thu Feb 20 15:44:40 2014 +0000
@@ -32,6 +32,23 @@
 #define LIST_RESOURCE "/incoming/aheinecke/test"
 #define SW_RESOURCE "/incoming/aheinecke/test"
 
+/** @brief A wrapper around a HINTERNET structure that handles closing
+ *
+ * Holds a HINTERNET structure and closes it if necessary on destruction.
+ *
+ */
+class SmartHINTERNET {
+public:
+    SmartHINTERNET() : handle(NULL) {}
+
+    ~SmartHINTERNET() {
+        if (handle) {
+            WinHttpCloseHandle(handle);
+        }
+    }
+
+    HINTERNET handle;
+};
 
 /** @brief Qt wrapper around FormatMessage
  *
@@ -224,11 +241,10 @@
 QDateTime Downloader::getLastModifiedHeader(HINTERNET hSession,
         HINTERNET hConnect, LPCWSTR resource)
 {
-    HINTERNET hRequest = NULL;
+    SmartHINTERNET sHRequest;
     SYSTEMTIME lMod;
     DWORD sizeOfSystemtime = sizeof (SYSTEMTIME);
     QDateTime retval;
-    DWORD err = 0;
 
     memset(&lMod, 0, sizeof (SYSTEMTIME));
 
@@ -237,65 +253,51 @@
         return retval;
     }
 
-    if (!createRequest(hSession, hConnect, &hRequest, L"HEAD",
+    if (!createRequest(hSession, hConnect, &sHRequest.handle, L"HEAD",
                 resource)) {
-        err = GetLastError();
-        goto cleanup;
+        return retval;
     }
 
-    if (!WinHttpSendRequest(hRequest,
+    if (!WinHttpSendRequest(sHRequest.handle,
                             WINHTTP_NO_ADDITIONAL_HEADERS,
                             0, WINHTTP_NO_REQUEST_DATA, 0,
                             0, 0)) {
-        err = GetLastError();
-        goto cleanup;
+        return retval;
     }
 
 
-    if (!WinHttpReceiveResponse(hRequest, NULL)) {
-        err = GetLastError();
-        goto cleanup;
+    if (!WinHttpReceiveResponse(sHRequest.handle, NULL)) {
+        return retval;
     }
 
-    if (!verifyCertificate(hRequest)) {
+    if (!verifyCertificate(sHRequest.handle)) {
         DEBUG << "Certificate verification failed";
-        goto cleanup;
+        return retval;
     }
 
-    if (!(WinHttpQueryHeaders(hRequest,
+    if (!(WinHttpQueryHeaders(sHRequest.handle,
                               WINHTTP_QUERY_LAST_MODIFIED |
                               WINHTTP_QUERY_FLAG_SYSTEMTIME,
                               NULL,
                               &lMod,
                               &sizeOfSystemtime,
                               WINHTTP_NO_HEADER_INDEX))) {
-        err = GetLastError();
-        goto cleanup;
+        return retval;
     }
 
     retval = QDateTime(QDate(lMod.wYear, lMod.wMonth, lMod.wDay),
                        QTime(lMod.wHour, lMod.wMinute, lMod.wSecond,
                              lMod.wMilliseconds),
                        Qt::UTC);
-
-// TODO remove and replace with a smart handle
-cleanup:
-    if (hRequest) {
-        WinHttpCloseHandle(hRequest);
-    }
-
-    // Close handle might overwrite the last error.
-    SetLastError(err);
     return retval;
 }
 
 bool Downloader::downloadFile(HINTERNET hSession, HINTERNET hConnect,
         LPCWSTR resource, const QString &fileName, DWORD maxSize)
 {
-    HINTERNET hRequest = NULL;
+    SmartHINTERNET sHRequest;
     bool retval = false;
     DWORD bytesAvailable = 0,
-          err = 0,
           bytesRead = 0,
           totalDownloaded = 0,
           contentLength = 0,
@@ -308,33 +310,30 @@
         return retval;
     }
 
-    if (!createRequest(hSession, hConnect, &hRequest, L"GET",
+    if (!createRequest(hSession, hConnect, &sHRequest.handle, L"GET",
                 resource)) {
-        err = GetLastError();
-        goto cleanup;
+        return retval;
     }
 
-    if (!WinHttpSendRequest(hRequest,
+    if (!WinHttpSendRequest(sHRequest.handle,
                             WINHTTP_NO_ADDITIONAL_HEADERS,
                             0, WINHTTP_NO_REQUEST_DATA, 0,
                             0, 0)) {
-        err = GetLastError();
-        goto cleanup;
+        return retval;
     }
 
 
-    if (!WinHttpReceiveResponse(hRequest, NULL)) {
-        err = GetLastError();
-        goto cleanup;
+    if (!WinHttpReceiveResponse(sHRequest.handle, NULL)) {
+        return retval;
     }
 
-    if (!verifyCertificate(hRequest)) {
+    if (!verifyCertificate(sHRequest.handle)) {
         DEBUG << "Certificate verification failed";
-        goto cleanup;
+        return retval;
     }
 
 
-    if (!(WinHttpQueryHeaders(hRequest,
+    if (!(WinHttpQueryHeaders(sHRequest.handle,
                               WINHTTP_QUERY_CONTENT_LENGTH |
                               WINHTTP_QUERY_FLAG_NUMBER,
                               NULL,
@@ -347,8 +346,7 @@
     }
 
     if (contentLength > maxSize) {
-        err = ERROR_INVALID_DATA;
-        goto cleanup;
+        return retval;
     }
 
     if (contentLength) {
@@ -359,15 +357,14 @@
             // No need to waste bandwidth.
             DEBUG << "Skipping download because file exists";
             retval = true;
-            goto cleanup;
+            return retval;
         }
     }
 
     // Open / Create the file to write to.
     if (!outputFile.open(QIODevice::WriteOnly)) {
         DEBUG << "Failed to open file";
-        err = GetLastError();
-        goto cleanup;
+        return retval;
     }
 
     DEBUG << "output file size: " << outputFile.size();
@@ -377,10 +374,9 @@
         memset(outBuf, 0, sizeof(outBuf));
         bytesRead = 0;
 
-        if (!WinHttpQueryDataAvailable(hRequest, &bytesAvailable)) {
+        if (!WinHttpQueryDataAvailable(sHRequest.handle, &bytesAvailable)) {
             DEBUG << "Querying for available data failed";
             retval = false;
-            err = GetLastError();
             break;
         }
 
@@ -392,21 +388,19 @@
         if (bytesAvailable > maxSize) {
             DEBUG << "File to large";
             retval = false;
-            err = ERROR_INVALID_DATA;
+            SetLastError(ERROR_INVALID_DATA);
             break;
         }
 
-        if (!WinHttpReadData(hRequest, (LPVOID)outBuf,
+        if (!WinHttpReadData(sHRequest.handle, (LPVOID)outBuf,
                              sizeof(outBuf), &bytesRead)) {
             DEBUG << "Error reading data";
-            err = GetLastError();
             break;
         } else {
             if (bytesRead) {
                 // Write data to file.
                 if (outputFile.write(outBuf, bytesRead) !=
                         bytesRead) {
-                    err = GetLastError();
                     DEBUG << "Error writing to file.";
                     retval = false;
                 }
@@ -431,27 +425,18 @@
         }
     } while (bytesAvailable > 0);
 
-// TODO remove and replace with a smart handle
-cleanup:
-
     if (retval && outputFile.isOpen()) {
         // Actually save the file to disk / move to homedir
         retval = outputFile.commit();
     }
 
-    if (hRequest) {
-        WinHttpCloseHandle(hRequest);
-    }
-
-    // Close handle might overwrite the last error.
-    SetLastError(err);
     return retval;
 }
 
 void Downloader::run() {
     bool results = false;
-    HINTERNET hSession = NULL,
-              hConnect = NULL;
+    SmartHINTERNET sHSession;
+    SmartHINTERNET sHConnect;
     wchar_t wUrl[mUrl.size() + 1];
     QDateTime lastModifiedSoftware;
     QDateTime lastModifiedList;
@@ -471,25 +456,25 @@
     wUrl[rc] = '\0';
 
     // Initialize connection
-    if (!openSession(&hSession)) {
+    if (!openSession(&sHConnect.handle)) {
         DEBUG << "Failed to open session: " << getLastErrorMsg();
         return;
     }
-    if (!initializeConnection(hSession, &hConnect, wUrl)) {
+    if (!initializeConnection(sHConnect.handle, &sHConnect.handle, wUrl)) {
         DEBUG << "Failed to initialize connection: " << getLastErrorMsg();
-        goto cleanup;
+        return;
     }
 
 
-    lastModifiedSoftware = getLastModifiedHeader(hSession, hConnect,
+    lastModifiedSoftware = getLastModifiedHeader(sHConnect.handle, sHConnect.handle,
             L""SW_RESOURCE);
 
-    lastModifiedList = getLastModifiedHeader(hSession, hConnect,
+    lastModifiedList = getLastModifiedHeader(sHConnect.handle, sHConnect.handle,
             L""LIST_RESOURCE);
 
     if (!lastModifiedList.isValid() || !lastModifiedSoftware.isValid()) {
         DEBUG << "Could not read headers: " << getLastErrorMsg();
-        goto cleanup;
+        return;
     }
 
     if (!mLastModSW.isValid() || lastModifiedSoftware > mLastModSW) {
@@ -497,7 +482,7 @@
 
         if (dataDirectory.isEmpty()) {
             DEBUG << "Failed to get data directory";
-            goto cleanup;
+            return;
         }
 
         QString fileName = dataDirectory.append("/SW-")
@@ -506,10 +491,10 @@
 
         DEBUG << "fileName: " << fileName;
 
-        if (!downloadFile(hSession, hConnect, L""SW_RESOURCE,
+        if (!downloadFile(sHConnect.handle, sHConnect.handle, L""SW_RESOURCE,
                    fileName, MAX_SW_SIZE)) {
             DEBUG << "Error downloading File: " << getLastErrorMsg();
-            goto cleanup;
+            return;
         }
 
         emit newSoftwareAvailable(fileName, lastModifiedSoftware);
@@ -518,7 +503,7 @@
 
         if (dataDirectory.isEmpty()) {
             DEBUG << "Failed to get data directory";
-            goto cleanup;
+            return;
         }
 
         QString fileName = dataDirectory.append("/list-")
@@ -527,10 +512,10 @@
 
         DEBUG << "fileName: " << fileName;
 
-        if (!downloadFile(hSession, hConnect, L""LIST_RESOURCE,
+        if (!downloadFile(sHConnect.handle, sHConnect.handle, L""LIST_RESOURCE,
                    fileName, MAX_LIST_SIZE)) {
             DEBUG << "Error downloading File: " << getLastErrorMsg();
-            goto cleanup;
+            return;
         }
 
         emit newListAvailable(fileName, lastModifiedList);
@@ -539,26 +524,12 @@
     DEBUG << "SW date: " << lastModifiedSoftware;
     DEBUG << "List date: " << lastModifiedList;
 
-    /*if (!WinHttpQueryDataAvailable(hRequest, &dataAvaiable)) {
-        DEBUG << "Failed to query data Available: " << getLastErrorMsg();
-        goto cleanup;
-    }*/
-
     if (!results) {
         // Report any errors.
         DEBUG << "Error" << GetLastError();
         emit error(tr("Unknown Problem when connecting"), ErrUnknown);
     }
 
-// TODO remove and replace with a smart handle
-cleanup:
-    if (hConnect) {
-        WinHttpCloseHandle(hConnect);
-    }
-
-    if (hSession) {
-        WinHttpCloseHandle(hSession);
-    }
     return;
 }
 #endif