CODEWITHSUNDEEP
c++winapi
user188757
user188757

Reputation: 123

WIN API ReadFile() returns GetLastError() ERROR_INVALID_PARAMETER

I wrote this code below and it worked fine under code::blocks using mingw gcc 4.7 to compile it. I have since decided to start using Visual Studio 2013 express. Now I am getting an error when ReadFile() is called. Which seems to be an invalid parameter. I can't see the error hoping someone here can spot it.

This is all wrapped inside a class Serial. From what I can see in the IDE the memory reference for m_hSerial is correct when compared to the reference CreateFile() returns to the handle.

m_hSerial = CreateFile(m_pchPort,
                       GENERIC_READ | GENERIC_WRITE,
                       0,
                       0,
                       OPEN_EXISTING,
                       FILE_FLAG_OVERLAPPED,
                       0);

I call the WorkThread like so

m_hThread = (HANDLE)_beginthreadex(0, 0, &WorkThread, (void*) this, 0, 0);

Here is the WorkThread code

unsigned int __stdcall Serial::WorkThread(void* pvParam)
{
// This is a pointer to the 'this' serial class.
// Needed to be able to set members of the class in a static class function
Serial * cThis = (Serial*) pvParam;
// Set up the overlapped event
OVERLAPPED ov;
memset(&ov, 0, sizeof(ov));
ov.hEvent = CreateEvent(0, true, 0, 0);
DWORD dwEventMask = 0;
DWORD dwWait;
HANDLE aHandles[2];
aHandles[0] = cThis->m_hThreadTerminator;
aHandles[1] = ov.hEvent;

SetEvent(cThis->m_hThreadRunning);
while (true)
{
    if (!WaitCommEvent(cThis->m_hSerial, &dwEventMask, &ov))
    {
        assert(GetLastError() == ERROR_IO_PENDING);

    }

    dwWait = WaitForMultipleObjects(2, aHandles, FALSE, INFINITE);
    switch(dwWait)
    {
        case WAIT_OBJECT_0:
        {
            _endthreadex(1);
        }
        case WAIT_OBJECT_0 + 1:
        {
            if (dwEventMask & EV_TXEMPTY)
            {
                ResetEvent(ov.hEvent);
            }
            else if (dwEventMask & EV_RXCHAR)
            {
                // read data here
                DWORD dwBytesRead = 0;
                DWORD dwErrors;
                COMSTAT cStat;
                OVERLAPPED ovRead;
                ovRead.hEvent = CreateEvent(0, true, 0, 0);

                // Get the Bytes in queue
                ClearCommError(cThis->m_hSerial, &dwErrors, &cStat);
                DWORD nSize = cStat.cbInQue;
                // EM_REPLACESEL needs a LPARAM null terminated string, make room and set the CString NULL
                char *szBuf = new char[nSize+1];
                memset(szBuf, 0x00, sizeof(szBuf));

                if (!ReadFile(cThis->m_hSerial, &szBuf, nSize, &dwBytesRead, &ovRead))
                    DWORD err = GetLastError();
                if (dwBytesRead == nSize)
                    SendMessage(cThis->m_hHwnd, WM_SERIAL, 0, LPARAM(&szBuf));

                CloseHandle(ovRead.hEvent); // clean up!!!
                delete[] szBuf;
            }
            // Reset the overlapped event
            ResetEvent(ov.hEvent);
        }
        break;
    }//switch
}

return 0;

}

Upvotes: 3

Views: 2419

Answers (1)

David Heffernan
David Heffernan

Reputation: 612784

ReadFile(cThis->m_hSerial, &szBuf, nSize, &dwBytesRead, &ovRead)

You are asking for an asynchronous operation, but also asking the function to tell you how many bytes have been read. You passed &dwBytesRead as the penultimate parameter. When you are performing overlapped reading, pass NULL for this parameter. The documentation says:

Use NULL for this parameter if this is an asynchronous operation to avoid potentially erroneous results.

It is also a mistake to pass &szBuf in the code above. You mean to pass szBuf.

You also fail to initialize the OVERLAPPED struct. Do so like this:

OVERLAPPED ovRead = {};

A bigger problem is that you ask for asynchronous access, but then write the code as it it were synchronous. As soon as ReadFile returns you attempt to get meaningful information out of dwBytesRead and you close the event that you put in the overlapped struct.

If you are really going code this asynchronously, you need to re-write the code to be asynchronous. On the face of it, it looks as though you have not fully understood the implications of overlapped I/O and you should perhaps switch to non-overlapped, synchronous I/O.

Upvotes: 5

Related Questions