Review: SerialPort FileOpen "FILE_FLAG_OVERLAPPED"

SoIntMan

Guten Morgen Leute,

ich möchte ein Geräte mit dem COM Port ansteuern. Das Gerät schickt mir daten auf Änderungen, und "gleichzetig" möchte ich dem Geräte Daten schicken. Somit verwende ich einen Lese Thread , und bei Eingabe schreibe ich was. Das ganze habe ich mal "schnell" so implmentiert, und habe ein Paar fragen (Pseudo):

Port öffnen

CreateFile(
       sw,
       GENERIC_WRITE | GENERIC_READ,
       0,
       NULL,
       OPEN_EXISTING,
       FILE_ATTRIBUTE_NORMAL | FILE_FLAG_OVERLAPPED,
       NULL
   );

   if (!GetCommState(*comHandle, &dcb))
       return false;

   dcb.BaudRate = CBR_9600; //9600 Baud
   dcb.ByteSize = 8; //8 data bits
   dcb.Parity = NOPARITY; //no parity
   dcb.StopBits = ONESTOPBIT; //1 stop

   if (!SetCommState(*comHandle, &dcb))
       return false;

OVERLAPPED _wait_o = { 0 }; // shared with thread loop
              _wait_o.hEvent = CreateEvent(NULL, TRUE, FALSE, NULL);

           if (_wait_o.hEvent == NULL)
           {
               return 0;
           }

Mein Lese Thread func (s == Atomic<bool>!):

void readLoop(string msg, LPOVERLAPPED _wait_o,  const HANDLE _serial_handle)
{
#define timeout 30000
    COMSTAT  stat;
    DWORD error, msk, length;
  
    SetCommMask(_serial_handle, EV_RXCHAR | EV_ERR);

    while (!s)
    {
        msk = 0;
        WaitCommEvent(_serial_handle, &msk, _wait_o); // non-block => OVERLAPPED MOE

        //Wait until comm has a event
        auto i = WaitForSingleObject(_wait_o->hEvent, INFINITE);

        if (i == WAIT_OBJECT_0)
        {
            ClearCommError(_serial_handle, &error, &stat);

            /* Exit loop */
            if (msk == 0 && s) 
            {
                break;
            }

            /* read data */
            if (msk & EV_RXCHAR)
            {
                ClearCommError(_serial_handle, &error, &stat);

                char* ReadBuf = new char[stat.cbInQue];
                DWORD actualReadLen;
                if (!ReadFile(_serial_handle, ReadBuf, stat.cbInQue, &actualReadLen, _wait_o))
                {
                    auto erro = GetLastError();
                    std::cout << "Read error: " << s << " " << erro << std::endl;
                    continue;
                }

                std::string s(ReadBuf, stat.cbInQue);
                delete[] ReadBuf;
                std::cout << "Read event: " << s << " " << stat.cbInQue << std::endl;
            }
        }
    }
    std::cout << "Exit thread\n" << std::endl;
}

Loop exit mit:

s = true;
SetCommMask(hPort, 0); // 0 msk == exit
readThread.join();

Schreiben daten:

bool res = WriteFile(hPort, s.c_str(), s.size(), &writtenBytes, &_wait_o);

Also funktionieren tut es, aber ich frage mich ob allgeimen noch was vergessen wurd? Sollte ich die WriteFile und ReadFile mit nem Lockguard versehen1?

Vielen Dank für konstruktive Kritik;)

Quiche Lorraine

@SoIntMan sagte in Review: SerialPort FileOpen "FILE_FLAG_OVERLAPPED":

/* read data */
            if (msk & EV_RXCHAR)
            {
                ClearCommError(_serial_handle, &error, &stat);

                char* ReadBuf = new char[stat.cbInQue];
                DWORD actualReadLen;
                if (!ReadFile(_serial_handle, ReadBuf, stat.cbInQue, &actualReadLen, _wait_o))
                {
                    auto erro = GetLastError();
                    std::cout << "Read error: " << s << " " << erro << std::endl;
                    continue;
                }

                std::string s(ReadBuf, stat.cbInQue);
                delete[] ReadBuf;
                std::cout << "Read event: " << s << " " << stat.cbInQue << std::endl;
            }

Aua, hier hast du ein Speicherloch bei der Variable ReadBuf. Tritt bei ReadFile ein Fehler auf, so gibst du die Variable ReadBuf nicht frei. Besser du nutzt std::string oder std::unique_ptr.

SoIntMan

@Quiche-Lorraine sagte in Review: SerialPort FileOpen "FILE_FLAG_OVERLAPPED":

Aua, hier hast du ein Speicherloch bei der Variable ReadBuf. Tritt bei ReadFile ein Fehler auf, so gibst du die Variable ReadBuf nicht frei. Besser du nutzt std::string oder std::unique_ptr.

ok super vielen dank, da muss ich als mal genauer hinschauen..

aber wie sieht es aus mit Read/WriteFile!? is das mit OVERLAPPT Threadsafe oder muss ich da mit mutex arbeiten
?