Formatting a packet with different types then send it over TCP

[SIFE]

I designed a simple protocol with this header:
V(1 byte)|L(4 bytes):T(1 byte):D(variable)
A real example will be like this packet:
0xH112250xT1dfsdffdfsdfdsfdsffdsfdfsdf...
So now to send it I do this:

#define PROTO 0xH1
#define TYPE 0xT1..


QByteArray msg;
QString query = "dfsfdsdfdsfdsf";
QDataStream out(&msg, QIODevice::WriteOnly);
out.setVersion(QDataStream::Qt_4_7);
out << PROTO << (quint32)0 << TYPE << query;
out.device()->seek(1);
out << quint32(query.length() - sizeof(quint32));

The compilation error message said:

guiclient.cpp:54: error: invalid suffix "xH1" on integer constant
So how I properly put hexadecimal in QDataStream?

[ChrisW67]

The error is exactly what the message says. 0xH1 is not a numeric literal in C++. Your compiler, trying to make the best of bad input, takes the leading zero as an int and then determines that xH1 is not a valid literal suffix.

You probably meant:


#define PROTO 0x1
#define PROTO 0x01
or
#define PROTO 1

They are equivalent. You must take action to ensure that integer use of these literals is the single byte you are expecting (it won't be by default).

You almost certainly do not want to use QDataStream unless the other end is a Qt program, and even then you should think twice. QDatastream does not dump the in-memory bytes to the stream, it serialises the objects concerned in object-defined ways. Just use the QIODevice methods directly to have direct control.

You should also avoid using the pre-processor for constant in C++ code:
http://www.parashift.com/c++-faq-lite/newbie.html#faq-29.11
http://www.parashift.com/c++-faq-lite/newbie.html#faq-29.8

[SIFE]

I am defining my own protocol messages, so I have many byte's like 0xH1. I took a look at emule source code, and in opcodes.h header I found the developer's define messages types like this:

#define EDONKEYVERSION 0x3C

#define KADEMLIA_VERSION1_46c 0x01 /*45b - 46c*/

#define KADEMLIA_VERSION2_47a 0x02 /*47a*/

#define KADEMLIA_VERSION3_47b 0x03 /*47b*/

#define KADEMLIA_VERSION5_48a 0x05 // -0.48a
So how do I make the compiler understand my own types like emule do?

You almost certainly do not want to use QDataStream unless the other end is a Qt program, and even then you should think twice. QDatastream does not dump the in-memory bytes to the stream, it serialises the objects concerned in object-defined ways. Just use the QIODevice methods directly to have direct control.
This is useful note, I didn't know it.

[ChrisW67]

So how do I make the compiler understand my own types like emule do?
Those things are not types, they are preprocessor symbols that are substituted literally wherever they appear after their declaration. You can define preprocessor symbols for constants exactly like emule does (and my previous reply indicates) if that is what you want. C++ has better methods for declaring a constant, but define works too.

None of the symbols in the eMule example are in the form 0xH1 because that has never been a valid C or C++ integer constant. "0x" introduces a hex constant but "H" is not a hex digit.

[SIFE]

You almost certainly do not want to use QDataStream unless the other end is a Qt program, and even then you should think twice. QDatastream does not dump the in-memory bytes to the stream, it serialises the objects concerned in object-defined ways. Just use the QIODevice methods directly to have direct control.
Do you have any snippets for that.
Now I am sending packet like this:

void GuiClient::sendMsg()
{
QByteArray msg;
QDataStream out(&msg, QIODevice::WriteOnly);
out.setVersion(QDataStream::Qt_4_7);
out << PROTO_VERSION << hex << (quint32)0 << MSG_TYPE << hex << ui->msgEdit->text().trimmed();
out.device()->seek(1);
out << quint32(msg.size() - sizeof(quint32));

qDebug("%d", msg.size());

client->write(msg);
//V1-L4-T1-D~
ui->msgEdit->clear();
ui->msgEdit->setFocus();
}
And to extract info from fields:

void GuiServer::readMsg()
{
if(client) //user stored member variable, should be 0 if no client is connected
{
QDataStream in(client);

quint32 blockSize = 0;

while (client->bytesAvailable() < (int)sizeof(quint32) + 1) {
if (!client->waitForReadyRead(2000)) {
//emit error(client->error(), client->errorString());
return;
}
}

unsigned char proto, msg_type;

in.setVersion(QDataStream::Qt_4_7);

in >> proto >> blockSize >> msg_type;

qDebug("%X", proto);
qDebug("%X", msg_type);
qDebug("%d", blockSize);

while (client->bytesAvailable() < blockSize) {
if (!client->waitForReadyRead(2000)) {
//emit error(socket.error(), socket.errorString());
return;
}
}

QString msg;

in >> msg;

ui->historyEdit->append(msg);
}

}

But this extract no proto, msg_type nor the msg except the size and time correctly and and time incorrectly.

[ChrisW67]

Here is your code in a self-contained example writing to a file:


#include <QtCore>
#include <QDebug>

#define PROTO_VERSION 1
#define MSG_TYPE 34
QString testMsg("This is a dummy message");

int main(int argc, char *argv[])
{
QCoreApplication app(argc, argv);

QByteArray msg;
QDataStream out(&msg, QIODevice::WriteOnly);
out.setVersion(QDataStream::Qt_4_7);
out << PROTO_VERSION << hex << (quint32)0 << MSG_TYPE << hex << testMsg;
out.device()->seek(1);
out << quint32(msg.size() - sizeof(quint32));


QFile f("/tmp/f.bin");
if (f.open(QIODevice::WriteOnly)) {
f.write(msg);
f.close();
}

return 0;
}


First is the obvious problem of trying to use QTextStream::hex() on a QDataStream... yes, the warning messages your compiler is issuing mean something:


main.cpp: In function ‘int main(int, char**)’:
main.cpp:15: warning: the address of ‘QTextStream& hex(QTextStream&)’ will always evaluate as ‘true’
main.cpp:15: warning: the address of ‘QTextStream& hex(QTextStream&)’ will always evaluate as ‘true’


You really need to understand the difference between a byte and the hexadecimal representation of that byte.
You need to re-read my earlier advice about forcing the size of integers. PROTO_VERSION is written as four bytes (and partially overwritten) and MSG_TYPE is also 4 bytes. The value "true" referred to in the warning is written as a single byte.

Look at the actual bytes you are sending by dumping the byte array. Inspect the stream byte-by-byte and then you will possibly understand why this is doomed to fail.

Dumped binary file for you to analyse


0000000 00 00 00 00 3c 00 00 00 00 00 00 00 22 01 00 00
nul nul nul nul < nul nul nul nul nul nul nul " soh nul nul
0000020 00 2e 00 54 00 68 00 69 00 73 00 20 00 69 00 73
nul . nul T nul h nul i nul s nul sp nul i nul s
0000040 00 20 00 61 00 20 00 64 00 75 00 6d 00 6d 00 79
nul sp nul a nul sp nul d nul u nul m nul m nul y
0000060 00 20 00 6d 00 65 00 73 00 73 00 61 00 67 00 65
nul sp nul m nul e nul s nul s nul a nul g nul e


The "00 00 00 3c" (60 decimal) is the block size you wrote at offset 1, partially over the PROTO_VAR value.
The "00 00 00 22" (34 decimal) in the first line is the MSG_TYPE.
QDataStream serialises the QString class as a 4-byte size followed by the characters in in 16-bit chars. The 23 character string has a size of 46 bytes, "00 00 00 2e" in the listing.

[SIFE]

I don't have any compiler warning, I forget to mention how PROTO_VERSION and MSG_TYPE how they are defined:

#define PROTO_VERSION 0xA7
#define MSG_TYPE 0xA8
In sendMsg SLOT, I can see 14 byte sent (12 bytes + 1 byte(PROTO_VERSION) + 1 byte(MSG_TYPE), with no text additional), but in the other side I don't know how to extract it correctly again.

[wysota]

Do you understand what QIODevice::seek() does? Do you understand the difference between QIODevice::write() and QDataStream's << operator? Do you understand how the compiler interprets number literals in code? What is the size (in bytes) of 0xA7 on a 32 bit machine?

[ChrisW67]

I don't have any compiler warning, I forget to mention how PROTO_VERSION and MSG_TYPE how they are defined:

Get a better compiler or turn on the warnings. The actual small values of PROTO_VERSION and MSG_TYPE are irrelevant to your problem.


In sendMsg SLOT, I can see 14 byte sent (12 bytes + 1 byte(PROTO_VERSION) + 1 byte(MSG_TYPE), with no text additional), but in the other side I don't know how to extract it correctly again.
As my example made with your code shows very clearly you are not sending anything like what you think you are sending. Before you use seek() and write the block size your buffer contains:

Four bytes for PROTO_VERSION
One byte 0x01 as a result of the "hex"
Four zero bytes
Four bytes for MSG_TYPE
One byte 0x01 as as result of the "hex"
Your serialised QString
Four bytes string length in bytes. If you send an empty string this will still be present.
Two bytes for each character in the string.


When you seek and write the block size you overwrite the last three bytes of 1 and the byte at 2. The smallest this block will be is 18 bytes.

Just because a number can fit into a single byte does not mean the compiler will do so. The default size of an integer literal is sizeof(int), typically four bytes although it could be 2 or 8.

[SIFE]

I have generic information:

Do you understand what QIODevice::seek() does?
Move cursor to a specified position in stream.

Do you understand the difference between QIODevice::write() and QDataStream's << operator?
Not exactly, but I know QDataStream serializing data.

Do you understand how the compiler interprets number literals in code?
Not sure.

What is the size (in bytes) of 0xA7 on a 32 bit machine?
1 Byte.

[ChrisW67]

I suggest that you look at your last two answers in light of the blow-by-blow deconstruction I did in my last post and this test program:


#include <iostream>
#define PROTO_VERSION 0xA7
int main(int argc, char *argv[]) {
std::cout << sizeof(PROTO_VERSION) << std::endl
<< sizeof(static_cast<short>(PROTO_VERSION)) << std::endl
<< sizeof(static_cast<unsigned char>(PROTO_VERSION)) << std::endl;
}

[wysota]

I have generic information:

Move cursor to a specified position in stream.
Yes, in bytes. So seek(1) moves to the second byte in the stream. Compare that to the last question I asked and my response to it together with what Christ has written.


Not exactly, but I know QDataStream serializing data.
But you don't want to serialize data.


Not sure.
It interprets them as integers.


1 Byte.
No. 0xA7 is really (usually) 0x000000A7 on a 32 bit machine and it occupies four bytes.

[SIFE]

No. 0xA7 is really (usually) 0x000000A7 on a 32 bit machine and it occupies four bytes.
I thought hexadecimal size is 4bits.
I Understand I have to cast PROTO_VERSION to unsigned char so I can make 1 byte, I make this changes:

void GuiClient::sendMsg()
{
QByteArray msg;
QDataStream out(&msg, QIODevice::WriteOnly);
out.setVersion(QDataStream::Qt_4_7);
out <<static_cast<unsigned char> (PROTO_VERSION)
<< (quint32)0 << static_cast<unsigned char>(MSG_TYPE) << ui->msgEdit->text().toAscii().trimmed();

out.device()->seek(1);
out << quint32(msg.size() - sizeof(quint32));

qDebug("%d", msg.size());

client->write(msg);

QFile f("test0" + QString::number(++i) + ".bin");
if (f.open(QIODevice::WriteOnly)) {
f.write(msg);
f.close();
}

ui->msgEdit->clear();
ui->msgEdit->setFocus();
}



void GuiServer::readMsg()
{
if(client) //user stored member variable, should be 0 if no client is connected
{
QDataStream in(client);

quint32 blockSize = 0;

while (client->bytesAvailable() < (int)sizeof(quint32) + 2) {
if (!client->waitForReadyRead(2000)) {
//emit error(client->error(), client->errorString());
return;
}
}

unsigned char proto, msg_type;

in.setVersion(QDataStream::Qt_4_7);

in >> proto;
in.device()->seek(1);
in >> blockSize;
in.device()->seek(1 + (int)sizeof(quint32));
in >> msg_type;

qDebug("protocol %X", proto);
qDebug("size %d", blockSize);
qDebug("message type %X", msg_type);

while (client->bytesAvailable() < blockSize) {
if (!client->waitForReadyRead(2000)) {
//emit error(socket.error(), socket.errorString());
return;
}
}

QString msg;

in.device()->seek(2 + (int)sizeof(quint32));

in >> msg;

ui->historyEdit->append(msg);
}

}
The dump show this:

$ hexdump -Cd test09.bin
00000000 a7 00 00 00 08 a8 00 00 00 02 2a 38 |..........*8|
0000000 00167 00000 43016 00000 00512 14378
000000c
What I miss here?

[wysota]

I thought hexadecimal size is 4bits.
What does it have to do with anything?


What I miss here?
You are still using QDataStream, which is not what you really want.

[ChrisW67]

out <<static_cast<unsigned char> (PROTO_VERSION)
<< (quint32)0 << static_cast<unsigned char>(MSG_TYPE)
<< ui->msgEdit->text().toAscii().trimmed();

out.device()->seek(1);
out << quint32(msg.size() - sizeof(quint32));

The dump show this:

$ hexdump -Cd test09.bin
00000000 a7 00 00 00 08 a8 00 00 00 02 2a 38 |..........*8|
0000000 00167 00000 43016 00000 00512 14378
000000c
What I miss here?

Nothing, it is outputting exactly what you told it to:
a7 = PROTO_VERSION
00 00 00 08 = block size (12) - sizeof(quint32)
a8 = MSG_TYPE
Serialised QByteArray:
00 00 00 02 = length of array in bytes
2a = '*'
38 = '8'

[giantdragon]

If you don't plan to send large binary data parts, JSON serialization will be the most convenient solution. Look at QJson library.

[SIFE]

What does it have to do with anything?
This change the image of how hexadecimal represent it in 32-bit machine.

You are still using QDataStream, which is not what you really want.
Would you suggest a solution fit my need, I made this try. It looks send the formatted packet as I expect but I don't now how to parse it correctly in other side:

void GuiClient::sendMsg()
{
QByteArray msg;

int n = ui->msgEdit->text().toAscii().trimmed().size();

msg.append(static_cast<unsigned char> (PROTO_VERSION));

for(int i2 = 0; i2 != sizeof(n); ++i2)
{
msg.append((char)(n&(0xFF << i2) >>i2));
}

msg.append(static_cast<unsigned char>(MSG_TYPE));
msg.append(ui->msgEdit->text().toAscii().trimmed());


qDebug("%d", msg.size());
qDebug("%s", msg.data());

client->write(msg);

QFile f("test0" + QString::number(++i) + ".bin");
if (f.open(QIODevice::WriteOnly)) {
f.write(msg);
f.close();
}

ui->msgEdit->clear();
ui->msgEdit->setFocus();
}


void GuiServer::readMsg()
{
if(client) //user stored member variable, should be 0 if no client is connected
{
int blockSize = 0;

while (client->bytesAvailable() < (int)sizeof(int) + 2) {
if (!client->waitForReadyRead(2000)) {
//emit error(client->error(), client->errorString());
return;
}
}

QByteArray array(client->read(6));

QByteArray array2;

array2[0] = array[1];
array2[1] = array[2];
array2[2] = array[3];
array2[3] = array[4];

memcpy(&blockSize, array2, sizeof(int));

int i = 0;

QFile f("test0" + QString::number(++i) + ".bin");
if (f.open(QIODevice::WriteOnly)) {
f.write(array);
f.close();
}

qDebug("msg %s", array.data());
qDebug("protocol %s", array.data()[0]);
qDebug("size %d", blockSize);
qDebug("message type %s", array.data()[5]);

while (client->bytesAvailable() < blockSize) {
if (!client->waitForReadyRead(2000)) {
//emit error(socket.error(), socket.errorString());
return;
}
}

QString msg;

ui->historyEdit->append(msg);
}
}
Hexdump shows:
Client side:
$ hexdump -Cb test01.bin

00000000 a7 05 05 05 05 a8 65 72 74 |......ert|
0000000 247 005 005 005 005 250 145 162 164
0000009

Server side:

00000000 a7 05 05 05 05 a8 |......|
0000000 247 005 005 005 005 250
0000006
It seems like the server can read the size of packet but it crashes when it tries to read data.
I am in this project since week(trying to implement my protocol), google tired from me, and I am not sleep well because this(its like nightmare).

[wysota]

This change the image of how hexadecimal represent it in 32-bit machine.
No, it doesn't. The datatype states it is a "32 bit integer", it has nothing to do with its hexadecimal representation. You can as well represent it as 32 binary digits. Hex, bin or dec are just interpretations of the actual data stored in the computer memory according to internal representation chosen by the CPU and its compiler.


I made this try.
A very good try. You should have done that in the beginning.

It looks send the formatted packet as I expect but I don't now how to parse it correctly in other side
Exactly the same way you assemble the message on the sending side.

If you send two bytes here, then you need to extract two bytes there, etc.


I am in this project since week(trying to implement my protocol), google tired from me, and I am not sleep well because this(its like nightmare).
Then take a piece of paper and draw a scheme of how your protocol is supposed to work and the bit-by-bit representation of its PDU. If you don't know how to do it, have a look at diagrams of standard binary protocols such as IP or NTP. If you google for "IP packet diagram", something meaningful will pop up. Practise with a raw stream of bytes until you learn to detect a beginning and end of each protocol unit in the raw stream. If you can't do it then probably the protocol lacks some necessary information and you have to adjust it accordingly.

Only when you fully understand how your protocol should work (and why this and not some other way), then you may start writing an implementation. If you expect to talk to computers with different architecture, pay special attention to the byte order of words.

[ChrisW67]

The binary dumps show the size figure as "05 05 05 05" (hex), or 84 215 045 bytes, with a sent text message length of just three bytes. Something is wrong with your sending logic (Hint: shift by bytes not bits). Perhaps you should research the htonl() and ntohl() macros/functions.

This thread is a reminder of why major high-level network protocols (SMTP, HTTP, NNTP, POP3, IMAP) are text-based and not binary. Not as efficient but a lot easier to get right.

[SIFE]

Then take a piece of paper and draw a scheme of how your protocol is supposed to work and the bit-by-bit representation of its PDU. If you don't know how to do it, have a look at diagrams of standard binary protocols such as IP or NTP. If you google for "IP packet diagram", something meaningful will pop up. Practise with a raw stream of bytes until you learn to detect a beginning and end of each protocol unit in the raw stream. If you can't do it then probably the protocol lacks some necessary information and you have to adjust it accordingly.

Only when you fully understand how your protocol should work (and why this and not some other way), then you may start writing an implementation. If you expect to talk to computers with different architecture, pay special attention to the byte order of words.
I already did that, now I am in the implementation, just because i am new to qt network programming.
Now I have a problem in receiving data:

void GuiServer::readMsg()
{
if(client) //user stored member variable, should be 0 if no client is connected
{
int blockSize = 0;

while (client->bytesAvailable() < (int)sizeof(int) + 2) {
if (!client->waitForReadyRead(2000)) {
//emit error(client->error(), client->errorString());
return;
}
}

QByteArray data(client->read(6));

QByteArray array2;
array2.reserve(4);
array2[0] = data[1];
array2[1] = data[2];
array2[2] = data[3];
array2[3] = data[4];

memcpy(&blockSize, array2, sizeof(int));

/* for (int j = 0; j < 4; ++j)
blockSize |= ((int)data[j] & 0x000000ff) << (j * 8);*/


QFile f("test0" + QString::number(++i) + ".bin");
if (f.open(QIODevice::WriteOnly)) {
f.write(data);
f.close();
}

qDebug("size %d", blockSize);
/*qDebug("msg %s", data.data());
qDebug("protocol %s", data.data()[0]);
qDebug("message type %s", data.data()[5]);
*/
while (client->bytesAvailable() < blockSize) {
if (!client->waitForReadyRead(2000)) {
return;
}
}

QString msg(data);

ui->historyEdit->append(msg);
}
}
6561
I sent six messages from client, they all sent correctly however in the are side it seems like I receive the length first and if i sent another message from the client I receive the body. The photo from attachment shows how the server receive each message.

[ChrisW67]

So what happens in the second while loop if all your bytes arrived at once and were available when you entered the routine? What about if only some of the message bytes have been received? What if some of the bytes are delayed?

Have you set a breakpoint and single-stepped this?

[SIFE]

The first loop to read 6 bytes(2 bytes + 4 bytes of size), in the second loop I block until I read the whole packet, so this all I get. If you have better solution please post it.

[wysota]

in the second loop I block until I read the whole packet
What if two packets arrive at once?


If you have better solution please post it.
My universal approach is to use signals and slots, buffer incoming data and process the buffer in a while loop as long as there is enough data in it.

[SIFE]

My universal approach is to use signals and slots, buffer incoming data and process the buffer in a while loop as long as there is enough data in it.
Would you say it in code :D.

[wysota]

If you want code then search the forum. I can give you a short explanation -- connect to the socket's readyRead() signal, append all pending data to your own buffer and process the buffer in a while loop according to the following pseudo-code logic:

buffer += socket.readAll();
while(buffer.size >= expectedBlockSize) {
data = buffer.take(expectedBlockSize);
process(data);
}
Of course expectedBlockSize will depend on what you expect to receive.