Hello everybody,
It’s nice you brought up this topic, Atul! I think some good things can be done in regard to prepareReadPayload(). The original idea was to separate validation of packet structure from reading the actual data in order to make the code for each more straightforward. Once the it is verified that the packet has valid structure we can rest assured that we read what we expect without causing a buffer overflow for example. Furthermore, because we read from QDataStream we can abstract from the byte order for example.
Initially all worked great but that was until I started to implement all possible Gnutella packet extensions. The reason is most of them are kind of hacks into the original 0.4 protocol and packet layout. For example at some place two nulls were written and an extension would add data between the nulls. I totally agree that a great deal of refactoring can be done in all prepareReadPayload() code. Another good example is when the GGEP extension block data is validated. One would often see:
const char *extensionRawData = rawPayload.constData() + MinimalPayloadLength;
int extensionSize = rawPayload.length() - MinimalPayloadLength;
QByteArray extensionData = QByteArray::fromRawData (extensionRawData, extensionSize);
d->ggepBlock.prepareRead (extensionData);
I can even think about creating a class (e.g. BufferValidator or DataValidator or DataValidationStream) that wraps the raw payload buffer and provides functions for verifying the structure of underlying data and automatically increases the current position (similarly to QDataStream). In that case it may even make sense to combine verification and actual reading in a single function.
Anybody else wanting to do some brainstorming on that?
Peter
P.S. I wouldn’t like to change too much in Gnutella::Packets just for the sake of experimentation. It would be better to think about how the design we have now can be improved and try to do a better job when implementing BitTorrent::Packets (yes, BitTorrent uses binary messages in peer transfer connections). It would be nice if we could be able to come up with an abstract Packet class from which e could derive the packets of concrete protocols.
P.S.2. Atul, I guess it was the html validator of Wordpress that caused your previous message to be truncated because of something in the source code you've pasted.