HTTP download with dynamic reallocation and retries This code is for downloading a file via winhttp lib, It's invoked by StartDownload function which tries to download n times before returning false if maximum tries exceeded. I'm worried about the dynamic realloc part in Download function, is my approach here have any flaws, or is there any improvements that can be made?

#include "HttpDownload.h"
#include &lt;Windows.h&gt;
#include &lt;Winhttp.h&gt;
#pragma comment(lib, "winhttp.lib")

bool bResults = FALSE;
HINTERNET hSession = 0, hConnect = 0, hRequest = 0;
int RepeatedTimes = 0;

void HttpDownload::CloseHandles() {
    if (hRequest) WinHttpCloseHandle(hRequest);
    if (hConnect) WinHttpCloseHandle(hConnect);
    if (hSession) WinHttpCloseHandle(hSession);
}

bool HttpDownload::InitHttp() {
    if (!(hSession = WinHttpOpen(0, 0, 0, 0, 0))) return false;
    if (!(hConnect = WinHttpConnect(hSession, L"127.0.0.1", 80, 0))) return false;
    if (!(hRequest = WinHttpOpenRequest(hConnect, L"GET", L"/file.ext", 0, 0, 0, 0))) return false;
    if (!(bResults = WinHttpSendRequest(hRequest, 0, 0, 0, 0, 0, 0))) return false;
    if (!(bResults = WinHttpReceiveResponse(hRequest, 0))) return false;
    return true;
}

bool HttpDownload::Download() {
    if (!bResults) return false;
    unsigned long dwSize = 0;
    unsigned long dwDownloaded = 0;
    void *buf;
    bool failed = false;
    int curalloc = 8192;
    unsigned char *jar = (unsigned char*)malloc(curalloc);
    int totalDownloaded = 0;
    #define failbreak failed = true; break;
    while (1) {
        dwSize = 0;
        if (!WinHttpQueryDataAvailable(hRequest, &amp;dwSize)) { failbreak }
        if (dwSize == 0) break;
        buf = VirtualAlloc(0, dwSize, MEM_COMMIT, PAGE_READWRITE);
        if (!buf) { failbreak }
        else {
            if (!WinHttpReadData(hRequest, buf, dwSize, &amp;dwDownloaded)) { failbreak }
            else {
                realloc:
                if ((totalDownloaded + dwDownloaded) &gt; curalloc) {
                    unsigned char *m = (unsigned char*)malloc(curalloc * 2);
                    curalloc = curalloc * 2;
                    memcpy(m, jar, totalDownloaded);
                    free(jar);
                    jar = m;
                    goto realloc;
                }
                memcpy((jar + totalDownloaded), buf, dwDownloaded);
                totalDownloaded += dwDownloaded;
            }
            VirtualFree(buf, 0, MEM_RELEASE);
        }
    } if (failed) return false;
    CloseHandles();
    FinalResult = (unsigned char*)malloc(totalDownloaded);
    memcpy(FinalResult, jar, totalDownloaded);
    free(jar);
    FinalDownloadedSize = totalDownloaded;
    return true;
}

bool HttpDownload::StartDownload() {
    CloseHandles();
    if (RepeatedTimes &gt; 4) { return false; }
    RepeatedTimes++;
    bool r = InitHttp();
    if (!r) StartDownload();
    r = Download();
    if (!r) StartDownload();
    return true;
}
at first StartDownload use recursion, when it absolute not need here. enough simply loop like - 

ULONG nTry = 4, dwError;
do {
    dwError = Download();
} while(dwError &amp;&amp; --Try);


next - you not need recreate all handles - hSession, hConnect, hRequest on send request fail, but only hRequest. the hSession we usual must open only once. the hConnect - once per url. note that WinHttpConnect really not connect to server. it simply remember server url and port in internal structures. if WinHttpOpen or WinHttpConnect - no sense try call it again (this is offline calls)

so code for synchronous (which is bad) processing must look like:

void test()
{
    if (HINTERNET hSession = WinHttpOpen( 0,  
        WINHTTP_ACCESS_TYPE_NO_PROXY,
        WINHTTP_NO_PROXY_NAME, 
        WINHTTP_NO_PROXY_BYPASS, 0))
    {
        if (HINTERNET hConnect = WinHttpConnect(hSession, L"stackoverflow.com", INTERNET_DEFAULT_HTTPS_PORT, 0))
        {
            int nTry = 4;
            ULONG dwError;
            do 
            {
                if (HINTERNET hRequest = WinHttpOpenRequest(hConnect, L"GET", NULL,
                    NULL, WINHTTP_NO_REFERER, 
                    WINHTTP_DEFAULT_ACCEPT_TYPES, 
                    WINHTTP_FLAG_REFRESH|WINHTTP_FLAG_SECURE))
                {
                    if (WinHttpSendRequest(hRequest,
                        WINHTTP_NO_ADDITIONAL_HEADERS,
                        0, WINHTTP_NO_REQUEST_DATA, 0, 
                        0, 0) &amp;&amp; WinHttpReceiveResponse(hRequest, 0))
                    {
                        dwError = Download(hRequest);
                    }
                    else
                    {
                        dwError = GetLastError();
                    }

                    WinHttpCloseHandle(hRequest);
                }
                else
                {
                    dwError = GetLastError();
                }

            } while (dwError &amp;&amp; --nTry);

            WinHttpCloseHandle(hConnect);
        }

        WinHttpCloseHandle(hSession);
    }
}


now look for you Download implementation - it contains mistakes and amazing not effective.

VirtualAlloc(0, dwSize, MEM_COMMIT, PAGE_READWRITE);


you always, unconditionally allocate temporary memory buffer (buf), even if you main buffer jar containing enough free space. than reallocate jar if need and copy temporary buffer to it memcpy((jar + totalDownloaded), buf, dwDownloaded);

also note that you have potential memory leak here - WinHttpReadData fail - you just break loop, without free temporary buf.

really we not need allocate any temporary buffer. we need just read data to main buffer jar - WinHttpReadData(hRequest, (PBYTE)jar + totalDownloaded, dwSize, &amp;dwSize). if jar not have enough free space - we need reallocate it before WinHttpReadData call.

now look how you reallocate jar - it's just inexplicable for me

realloc: ** goto realloc;

for what this loop ?!? why need curalloc = curalloc * 2; and multiple time allocate, copy, free data ?!?. when we can just set 

curalloc = totalDownloaded + dwDownloaded;

more better of course allocate some more memory than totalDownloaded + dwDownloaded. so code for Download can look like:

ULONG Download(HINTERNET hRequest)
{
    ULONG curalloc = 0x10000, dwSize, totalDownloaded = 0, cbNeed;

    HANDLE hHeap = GetProcessHeap();

    ULONG dwError = NOERROR;

    if (PVOID jar = HeapAlloc(hHeap, 0, curalloc))
    {
        do 
        {
            if (!WinHttpQueryDataAvailable(hRequest, &amp;dwSize))
            {
                dwError = GetLastError();
                break;
            }

            cbNeed = totalDownloaded + dwSize;

            if (cbNeed &gt; curalloc)
            {
                cbNeed = (cbNeed + 0xffff) &amp; ~0xffff;// round to 64kb

                if (PVOID buf = HeapReAlloc(hHeap, 0, jar, cbNeed))
                {
                    curalloc = cbNeed, jar = buf;
                }
                else
                {
                    dwSize = ERROR_OUTOFMEMORY;
                    break;
                }
            }

            if (!WinHttpReadData(hRequest, (PBYTE)jar + totalDownloaded, dwSize, &amp;dwSize))
            {
                dwError = GetLastError();
                break;
            }

            totalDownloaded += dwSize;

        } while (dwSize);

        if (!dwError)
        {
            DbgPrint("download %u Ok\n", totalDownloaded);
        }

        HeapFree(hHeap, 0, jar);
    }

    return GetLastError();
}


this is already ok, but not best. windows let reserve memory space. so good solution - at begin reserve large enough memory space (in 32bit we can reserve how minimum n*10Mb, in 64bit - gigabytes). and then allocate memory from this reserved range. with this we not need reallocate and copy data. class for this dynamic allocation:

class DynamicBuffer
{
    PBYTE _BaseAddress;
    SIZE_T _dwReserve, _dwSize, _dwCommit;

    static SIZE_T RoundSize(SIZE_T size)
    {
        static SIZE_T s_dwAllocationGranularity;

        if (!s_dwAllocationGranularity)
        {
            SYSTEM_INFO si;
            GetSystemInfo(&amp;si);
            s_dwAllocationGranularity = si.dwAllocationGranularity - 1;
        }

        return (size + s_dwAllocationGranularity) &amp; ~s_dwAllocationGranularity;
    }

public:

    DynamicBuffer()
    {
        _BaseAddress = 0, _dwReserve = 0, _dwSize = 0, _dwCommit = 0;
    }

    ~DynamicBuffer()
    {
        Reset();
    }

    ULONG Create(SIZE_T dwSize)
    {
        if (_BaseAddress = (PBYTE)VirtualAlloc(0, dwSize = RoundSize(dwSize), MEM_RESERVE, PAGE_READWRITE))
        {
            _dwReserve = dwSize;
            return NOERROR;
        }

        return GetLastError();
    }

    ULONG AllocBuffer(PVOID* ppv, SIZE_T cb)
    {
        if (_dwReserve - _dwSize &lt; cb)
        {
            return ERROR_OUTOFMEMORY;
        }

        SIZE_T dwSize = _dwSize + cb;

        if (dwSize &gt; _dwCommit)
        {
            SIZE_T dwCommit = RoundSize(dwSize);

            if (!VirtualAlloc(_BaseAddress + _dwCommit, dwCommit - _dwCommit, MEM_COMMIT, PAGE_READWRITE))
            {
                return GetLastError();
            }

            _dwCommit = dwCommit;
        }

        *ppv = _BaseAddress + _dwSize;

        return NOERROR;
    }

    void AddData(SIZE_T cb)
    {
        _dwSize += cb;

        if (_dwSize &gt; _dwCommit)
        {
            __debugbreak();
        }
    }

    PVOID getData()
    {
        return _BaseAddress;
    }

    SIZE_T getDataSize()
    {
        return _dwSize;
    }

    SIZE_T getFreeSpace()
    {
        return _dwReserve - _dwSize;
    }

    void Reset()
    {
        if (_BaseAddress)
        {
            VirtualFree(_BaseAddress, 0, MEM_RELEASE);
            _BaseAddress = 0;
        }
        _dwReserve = 0, _dwSize = 0, _dwCommit = 0;
    }
};


with this we can rewrite Download :

ULONG Download(HINTERNET hRequest)
{
    DynamicBuffer buf;

    ULONG dwError = NOERROR;

    if (dwError = buf.Create(0x4000000)) // 64Mb reserve
    {
        return dwError;
    }

    ULONG dwSize;

    do 
    {
        if (!WinHttpQueryDataAvailable(hRequest, &amp;dwSize))
        {
            dwError = GetLastError();
            break;
        }

        PVOID pv;
        if (dwError = buf.AllocBuffer(&amp;pv, dwSize))
        {
            break;
        }

        if (!WinHttpReadData(hRequest, pv, dwSize, &amp;dwSize))
        {
            dwError = GetLastError();
            break;
        }

        buf.AddData(dwSize);

    } while (dwSize);

    if (!dwError)
    {
        DbgPrint("download %u Ok\n", buf.getDataSize());
    }

    return GetLastError();
}


but however always much better do asynchronous download. this can be done say in next way:

class __declspec(novtable) InternetHandle
{
    HINTERNET _hInternet;
    LONG _dwRef;

protected:

    virtual ~InternetHandle()
    {
        if (_hInternet)
        {
            WinHttpCloseHandle(_hInternet);
        }
        DbgPrint("%s&lt;%p&gt;\n", __FUNCTION__, this);
    }

    InternetHandle()
    {
        _dwRef = 1;
        _hInternet = 0;
        DbgPrint("%s&lt;%p&gt;\n", __FUNCTION__, this);
    }

public:

    void AddRef()
    {
        InterlockedIncrement(&amp;_dwRef);
    }

    void Release()
    {
        if (!InterlockedDecrement(&amp;_dwRef)) delete this;
    }

    HINTERNET get_handle() { return _hInternet; }

    HINTERNET set_handle(HINTERNET hInternet) { 
        return InterlockedExchangePointer(&amp;_hInternet, hInternet);
    }
};

class CSession : public InternetHandle
{
public:
    ULONG Open(LPCWSTR pszAgentW = 0)
    {
        if (HINTERNET hSession = WinHttpOpen( pszAgentW,  
            WINHTTP_ACCESS_TYPE_NO_PROXY,
            WINHTTP_NO_PROXY_NAME, 
            WINHTTP_NO_PROXY_BYPASS, WINHTTP_FLAG_ASYNC))
        {
            set_handle(hSession);

            return NOERROR;
        }

        return GetLastError();
    }
};

class CTarget : public InternetHandle
{
    CSession* _pSession;

    virtual ~CTarget()
    {
        _pSession-&gt;Release();
    }
public:

    ULONG Set(PCWSTR pswzServerName, INTERNET_PORT nServerPort)
    {
        if (HINTERNET hConnect = WinHttpConnect(_pSession-&gt;get_handle(), pswzServerName, nServerPort, 0))
        {
            set_handle(hConnect);
            return NOERROR;
        }

        return GetLastError();
    }

    CTarget(CSession* pSession) : _pSession(pSession)
    {
        pSession-&gt;AddRef();
    }
};

class DownloadCtx : public InternetHandle, public DynamicBuffer, public WINHTTP_ASYNC_RESULT
{
    CTarget* _pTarget;
    ULONG _nTryCount;
    ULONG _dwThreadId;

    virtual ~DownloadCtx()
    {
        DbgPrint("&lt;%u,%u&gt; %u\n", dwResult, dwError, getDataSize());

        _pTarget-&gt;Release();

        PostThreadMessage(_dwThreadId, WM_QUIT, dwResult, dwError);
    }

public:
    DownloadCtx(CTarget* pTarget, ULONG nTryCount) : _pTarget(pTarget), _nTryCount(nTryCount)
    {
        pTarget-&gt;AddRef();
        dwError = ERROR_IO_PENDING, dwResult = 0;
        _dwThreadId = GetCurrentThreadId();
    }

    ULONG SendRequest()
    {
        if (get_handle()) __debugbreak();

        if (!dwError)
        {
            return NOERROR;
        }

        if (!_nTryCount--)
        {
            return dwError;
        }

        dwError = ERROR_IO_PENDING, dwResult = 0;

        Reset();

        DbgPrint("=========== %x ============\n", _nTryCount);

        if (HINTERNET hRequest = WinHttpOpenRequest(_pTarget-&gt;get_handle(), L"GET", NULL,
            NULL, WINHTTP_NO_REFERER, 
            WINHTTP_DEFAULT_ACCEPT_TYPES, 
            WINHTTP_FLAG_REFRESH|WINHTTP_FLAG_SECURE))
        {
            PVOID Context = this;

            if (WinHttpSetOption(hRequest, WINHTTP_OPTION_CONTEXT_VALUE, &amp;Context, sizeof(Context)))
            {
                AddRef();

                set_handle(hRequest);

                WinHttpSetStatusCallback(
                    hRequest,
                    _StatusCallback,
                    WINHTTP_CALLBACK_FLAG_ALL_NOTIFICATIONS,
                    NULL );

                if (!WinHttpSendRequest(hRequest,
                    WINHTTP_NO_ADDITIONAL_HEADERS,
                    0, WINHTTP_NO_REQUEST_DATA, 0, 
                    0, (DWORD_PTR)this))
                {
                    Close(GetLastError(), API_SEND_REQUEST);
                }

                return NOERROR;
            }

            WinHttpCloseHandle(hRequest);
        }

        return GetLastError();
    }

    ULONG Read(HINTERNET hRequest)
    {
        if (DWORD dwNumberOfBytesToRead = (DWORD)min(0x10000, getFreeSpace()))
        {
            PVOID Buf;
            if (ULONG err = AllocBuffer(&amp;Buf, dwNumberOfBytesToRead))
            {
                return err;
            }

            DbgPrint("Begin Read %u\n", dwNumberOfBytesToRead);

            return WinHttpReadData(hRequest, Buf, 0x10000, 0) ? NOERROR : GetLastError();
        }

        return ERROR_BUFFER_OVERFLOW;
    }

    void Close(ULONG err, DWORD_PTR from)
    {
        if (HINTERNET hRequest = set_handle(0))
        {
            dwError = err, dwResult = from;
            WinHttpCloseHandle(hRequest);
        }
    }

    static void CALLBACK _StatusCallback(
        __in  HINTERNET hRequest,
        __in  DWORD_PTR Context,
        __in  DWORD dwInternetStatus,
        __in  LPVOID lpvStatusInformation,
        __in  DWORD dwStatusInformationLength
        )
    {
        reinterpret_cast&lt;DownloadCtx*&gt;(Context)-&gt;StatusCallback(
            hRequest, dwInternetStatus, lpvStatusInformation, dwStatusInformationLength);
    }

    void StatusCallback(
        __in  HINTERNET hRequest,
        __in  DWORD dwInternetStatus,
        __in  LPVOID lpvStatusInformation,
        __in  DWORD dwStatusInformationLength
        );
};

void DownloadCtx::StatusCallback(
                                      __in  HINTERNET hRequest,
                                      __in  DWORD dwInternetStatus,
                                      __in  LPVOID lpvStatusInformation,
                                      __in  DWORD dwStatusInformationLength
                                      )
{
    DbgPrint("%x&gt;%p %08x %p %x\n", GetCurrentThreadId(), hRequest,
        dwInternetStatus, lpvStatusInformation, dwStatusInformationLength);

    ULONG Error;
    DWORD_PTR Result;

    switch (dwInternetStatus)
    {
    default: return;

    case WINHTTP_CALLBACK_STATUS_HANDLE_CLOSING:
        SendRequest();
        Release();
        return;

    case WINHTTP_CALLBACK_STATUS_REQUEST_ERROR:
        Error = reinterpret_cast&lt;WINHTTP_ASYNC_RESULT*&gt;(lpvStatusInformation)-&gt;dwError;
        Result = reinterpret_cast&lt;WINHTTP_ASYNC_RESULT*&gt;(lpvStatusInformation)-&gt;dwResult;
        break;

    case WINHTTP_CALLBACK_STATUS_SENDREQUEST_COMPLETE:
        if (WinHttpReceiveResponse(hRequest, NULL))
        {
            return;
        }

        Error = GetLastError(), Result = API_RECEIVE_RESPONSE;
        break;

    case WINHTTP_CALLBACK_STATUS_READ_COMPLETE:

        DbgPrint("READ_COMPLETE:%u\n",dwStatusInformationLength);

        if (dwStatusInformationLength)
        {
            AddData(dwStatusInformationLength);

            if (Error = Read(hRequest))
            {
                Result = API_READ_DATA;
                break;
            }
            return ;
        }

        Error = NOERROR, Result = 0;
        break;

    case WINHTTP_CALLBACK_STATUS_HEADERS_AVAILABLE:

        static volatile UCHAR guz;
        union {
            PWSTR sz;
            PVOID buf;
        };
        PVOID stack = _alloca(guz);
        ULONG cb = 0, rcb = 32;
        do 
        {
            if (cb &lt; rcb)
            {
                rcb = cb = (ULONG)((PBYTE)stack - (PBYTE)(buf = _alloca(rcb - cb)));
            }

            if (WinHttpQueryHeaders(hRequest, WINHTTP_QUERY_CONTENT_LENGTH, 0, buf, &amp;rcb, 0))
            {
                Error = NOERROR;

                if (cb = wcstoul(sz, &amp;sz, 10))
                {
                    DbgPrint("Content-Length: %u\n", cb);
                    cb++;
                }
                else
                {
                    Error = ERROR_NO_DATA;
                }
                break;
            }
        } while ((Error = GetLastError()) == ERROR_INSUFFICIENT_BUFFER);

        Result = 0;

        switch (Error)
        {
        case ERROR_WINHTTP_HEADER_NOT_FOUND:
            cb = 0x4000000;// 64 mb reserve
        case NOERROR:
            if (Error = Create(cb))
            {
                break;
            }

            if (Error = Read(hRequest))
            {
                Result = API_READ_DATA;
                break;
            }

            return ;
        }
        break;
    }

    Close(Error, Result);
}

void winhttp()
{
    if (CSession* pSession = new CSession)
    {
        if (!pSession-&gt;Open())
        {
            if (CTarget* pTarget = new CTarget(pSession))
            {
                if (!pTarget-&gt;Set(L"stackoverflow.com", INTERNET_DEFAULT_HTTPS_PORT))
                {
                    if (DownloadCtx* Ctx = new DownloadCtx(pTarget, 4))
                    {
                        Ctx-&gt;SendRequest();

                        Ctx-&gt;Release();
                    }
                }
                pTarget-&gt;Release();
            }
        }

        pSession-&gt;Release();
    }

    MessageBoxW(0,0,L"for demo",0);
}