No, your changes are fine! Sorry I failed to mention that! Please send me a patch to apply in the mainstream. Another thing I failed to mention in the previous post is that your patch would fix a bug I just noticed yesterday. You see, currentPos is increased with 8 in the body of the for loop and it is not verified if that doesn’t make currentPos > payloadLength. In that case no file name will be found but false will ONLY be returned if they are equal:

currentPos += 8; // fileIndex and fileSize
int fileNameStart = currentPos;
for (; currentPos < payloadLength; currentPos++)
if (rawPayload.at (currentPos) == 0)
		break;

// Found the '' for the fileName?
if (currentPos == payloadLength) // should have been currentPos >= payloadLength
return false;

Since you use indexOf(), a start position beyond the end of the buffer will result in -1 and preapreaReadPayload() will return false.

I’m quite sure your function and similar ones can be used on other places as well so we should make sure we reuse them! That’s why I thought it might be more intuitive to use such functions if they were members of a validator class for example.

The changes I meant are changes that totally change the way packets are parsed: combining verification and reading in one function, or using a special ValidatorDataStream in prepareReadPayload(). I would like to try a new, hopefully better parsing mechanism when implementing BitTorrent packets. Any ideas in this respect?

I’m not familiar with CppUnit, but I’ll make sure I take a look at it. I do want to able to develop and test each package and component separately from the others. I believe that will make development and debugging much easier. Some attempts to do that can be found in the packages Gnutella::Handshaking and Http. Any further ideas are welcome!

atul wrote:…

Peter Dimov wrote:

…..

P.S. I wouldn’t like to change too much Gnutella::Packets just for the sake of experimentation.

….

Are you referring to the changes ( or kind of changes ) like the ones I proposed…

If so, I understand ;-) It is usually a tight rope walk…

It really all depends on the tests… Exhaustive unit test suite which will tell me right away if I broke something…

What about CppUnit? CppUnit is a beauty… I started to pull Caltiko code into a test harness, but thought I would ask here first…

Separating out classes, and putting Unit Tests in is not simple but gives you a greater understanding… ( I could run some complex method into gdb and experiment with various input …)

I have a copy of Mike Feather’s book and used some of the techniques he proposes.. painful, yes but invaluable ;-)

—- cheers atul