QTcpSocket Read/Write correctly & reliable

[Narutoblaze]

Note : [I do not have control of the server that i am connecting to.]

How Server Sends Data :
8 Byte Binary 'BigEndian' Encoded Size Uint64
Then Rest Of The Data Is Raw.

Problems :

1. How do i verify that the 8 byte i am reading is the size not any other random data.

2. How do i correctly read the data and verify it has read full.

What I Tried :
connected readyread with signal and slot below is the slot.

void conn::readtcpsocket()
{
    qDebug() << "[*] Reading From Socket : " << socketDescriptor();

    QByteArray byte;
    if (bytesAvailable() > 8) {
        bool isOk;
        qulonglong size;

        size = read(8).toHex().toULongLong(&isOk, 16);
        qDebug() << "[*] Data Size : " << size;

        int totalread = 0;
        while (totalread != size) {
            if (waitForReadyRead() || isReadable()) {
                byte.append(read(bytesAvailable()));
                totalread += byte.size();
            }
        }

        qDebug() << "[*] Data Recived : " << byte.size();
        emit fullData(byte);
    }
}

Issue With This Code :

1. Sometimes i am reading some random 8 bytes which is causing some random number.

2. Even though i am checking totalread != size in while loop sometimes i end up reading more data then i supposed to or even few bytes less then i am supposed to.

3. Noticeable slow performance when large data.

Any help is appreciated, Thank you for your time.

[SGaist]

Hi,

How are you supposed to request the data from the server ?

[JonB]

@Narutoblaze
This kind of question gets asked many times. If you search the forum you can find similar questions.

Best is to get rid of while loops and waitForReadyRead()s. Each time readyRead() signal is emitted append new bytes to buffer. Take whatever bytes your "protocol" expects out of the buffer when it has received enough bytes to satisfy; leave any remainder for future.

Even though i am checking totalread != size in while loop sometimes i end up reading more data then i supposed to

When you read(bytesAvailable()) it may read further bytes from the next "message".

                byte.append(read(bytesAvailable()));
                totalread += byte.size();

This code, called in your loop, miscalculates totalread if called more than once, leading to totalread being set to more bytes than actually read. Also while (totalread != size) may never be true consequently, so it could loop forever. Think about the code (e.g. work through it if bytes happen to arrive 1 at a time and see what happens to totalread), it's wrong.

[Narutoblaze]

@SGaist it has commands like this

Commands :*
Byte 0x01 = Recent File
Byte 0x02 = Recent Modified File

...etc

[Narutoblaze]

@JonB I have looked into QDataStream and i believe it also expect the server to send data using QDataStream so i did not try it.

I tried this loop from QAbstractSocket Class Documention but still it reads up more data then it should and is not exiting when numtotalread = size

if (bytesAvailable() > 8) {
        bool isOk;
        qulonglong h;

        size = read(8).toHex().toULongLong(&isOk, 16);
        qDebug() << "[*] Data Size : " << h;

        int numRead = 0, numReadTotal = 0;
        char buffer[size];

        forever {
            numRead = read(buffer, size);
            numReadTotal += numRead;

            if (numRead == 0 && !waitForReadyRead() || numReadTotal == size) {
                break;
            }
        }

        qDebug() << numReadTotal << " = " << sizeof(buffer);
    }

[JonB]

@Narutoblaze said in QTcpSocket Read/Write correctly & reliable:

but still it reads up more data then it should

Prove this.

it should and is not exiting when numtotalread = size

So put necessary debugging statements in to find out what is going on.

[Narutoblaze]

@JonB I have also done little bit of digging of what server is sending for the command so server is sending 8byte size then file , 8byte size then file the first one is 200017bytes and second one is 200bytes my code reads the size and falling in the loop it does end up reading the first file then it gets stuck it waitForReadyRead() for 30 second its default timeout and then breaks from the loop and you can see the totalread is exactly 208bytes larger so it has read second file with the size.

void conn::readtcpsocket()
{
    qDebug() << "[*] Reading From Socket : " << socketDescriptor();
    if (bytesAvailable() > 8) {
        bool isOk;
        qulonglong size;

        size = read(8).toHex().toULongLong(&isOk, 16);
        qDebug() << "[*] Data Size : " << h;

        int numRead = 0, numReadTotal = 0;
        char buffer[size];

        forever {
            numRead = read(buffer, size);
            numReadTotal += numRead;

            qDebug() << "[*] Read : " << numRead;
            if (numRead == 0 && !waitForReadyRead() || numReadTotal == size) {
                break;
            }
        }

        qDebug() << "[*] Total Read : " << numReadTotal << " [*] Size Of Buffer : " << sizeof(buffer);
    }

    qDebug() << "END OF FUNCTION !";
}

Debug Output :

[*] Reading From Socket :  1336
[*] Data Size :  200017
[*] Read :  65528
[*] Read :  0
[*] Read :  65536
[*] Read :  0
[*] Read :  65536
[*] Read :  0
[*] Read :  3625
[*] Read :  0
[*] Total Read :  200225  [*] Size Of Buffer :  200017
END OF FUNCTION !

[JonB]

@Narutoblaze
Your code/algorithm is incorrect. This is basic coding, please study what your code does. You "actively" encourage reading across file boundaries, i.e. what you call "too much").

You (seem to) start by setting size to the total number of bytes expected for the file. Then you call read(buffer, size);. But when that returns with fewer bytes than size you still call it again next time round asking it to read up to size further bytes. This can/will exceed the number of bytes in the first file. You obviously need to reduce what size you pass to read() accordingly on subsequent calls. And then be careful what you compare in your if (... || numReadTotal == size). Adjust your code however to make it right.

I previously said that the whole synchronous approach of repeatedly calling waitForReadyRead() in a loop is not the event-driven way to do things. Allow bytes to arrive in repeated readyRead() signals asynchronously instead.