std::vector<char> troubles

[Chris Thompson]

Hi

I'm writing a p2p client for an existing protocol.

I used a std::vector<char> as a buffer for messages read from the server.
The message length is the first 4 bytes. The message code the second 4.
The total message length is therefore 4 + message length.

A number of messages work fine/as expected but there are consistant
errors occuring. After a period
the message lengths and codes are very large numbers. Another important
(i think) point is that in the while loop, bytes
are only removed from the vector if a whole message exists. Yet on
occasion the messageLength calculated changes
between itterations that do not remove any data? how can this be so?

Is it maybe the case that i should be using std::vector<unsigned char>
to store the data?

The code i suspect is to blame is below. I can't work out if memory is
getting overwritten/used uninitialised. As i understand it
the erase() call is fine so each complete message is removed correctly.

Any help very welcome as i'm stuck as a big stuck thing at the minute.
Chris

// inbByteBuffer = std::vector<char>
| // if we have got the message length and code bytes
while (inbByteBuffer.size
<http://www.php.net/manual-lookup.php?lang=en&pattern=size>() >= 8) {
int messageLength = 0;
int msgCode = 0;

// get the message length (first 4)
for(int i=3; i>=0; i--) {
messageLength*=256;
messageLength+= inbByteBuffer[i];
}

// get the message code (second 4)
for(int i=7; i>=4; i--) {
msgCode *=256;
msgCode += inbByteBuffer[i];
}

// check message is long enough
if ((messageLength+4) > inbByteBuffer.size
<http://www.php.net/manual-lookup.php?lang=en&pattern=size>()) {
return false;
}

// create the bytes specific to this message
std::vector<char> data;
for(int i=0; i<messageLength+4; i++) {
char c = inbByteBuffer[i];
data.push_back
<http://www.php.net/manual-lookup.php?lang=en&pattern=push_back>(c);
}

// the buffer is the correct size, create a message with it
try {
Message * pmsg = MessageFactory::buildMessage
<http://www.php.net/manual-lookup.php?lang=en&pattern=buildMessage>(data);

// add it to the inbound queue
inbMessageQueue.push
<http://www.php.net/manual-lookup.php?lang=en&pattern=push>(pmsg);
} catch
<http://www.php.net/manual-lookup.php?lang=en&pattern=catch>
(logic_error e) {
cout << "Invalid message exception" << endl;
}

// get the iterator
std::vector<char>::iterator it = inbByteBuffer.begin
<http://www.php.net/manual-lookup.php?lang=en&pattern=begin>();

// remove the items from the inbByteBuffer
for(int i=0; i<= messageLength+4 && inbByteBuffer.begin
<http://www.php.net/manual-lookup.php?lang=en&pattern=begin>() !=
inbByteBuffer.end
<http://www.php.net/manual-lookup.php?lang=en&pattern=end>();i++) {
it = inbByteBuffer.begin
<http://www.php.net/manual-lookup.php?lang=en&pattern=begin>();
inbByteBuffer.erase
<http://www.php.net/manual-lookup.php?lang=en&pattern=erase>(it);
}
}

|

[John Harrison]

"Chris Thompson" <ch************@tascomi.com> wrote in message
news:10*************@corp.supernews.com...

Hi

I'm writing a p2p client for an existing protocol.

I used a std::vector<char> as a buffer for messages read from the server.
The message length is the first 4 bytes. The message code the second 4.
The total message length is therefore 4 + message length.

A number of messages work fine/as expected but there are consistant
errors occuring. After a period
the message lengths and codes are very large numbers. Another important
(i think) point is that in the while loop, bytes
are only removed from the vector if a whole message exists. Yet on
occasion the messageLength calculated changes
between itterations that do not remove any data? how can this be so?

Is it maybe the case that i should be using std::vector<unsigned char>
to store the data?
Hard to say, but if not you should certainly cast to unsigned char in your
msgLength and msgCode calculations.

// get the message length (first 4)
for(int i=3; i>=0; i--) {
messageLength*=256;
messageLength+= static_cast<unsigned char>(inbByteBuffer[i]);
}

The code i suspect is to blame is below. I can't work out if memory is
getting overwritten/used uninitialised. As i understand it
the erase() call is fine so each complete message is removed correctly.

Any help very welcome as i'm stuck as a big stuck thing at the minute.
Chris

[snip]

// get the iterator
std::vector<char>::iterator it = inbByteBuffer.begin
<http://www.php.net/manual-lookup.php?lang=en&pattern=begin>();

// remove the items from the inbByteBuffer
for(int i=0; i<= messageLength+4 && inbByteBuffer.begin
<http://www.php.net/manual-lookup.php?lang=en&pattern=begin>() !=
inbByteBuffer.end
<http://www.php.net/manual-lookup.php?lang=en&pattern=end>();i++) {
it = inbByteBuffer.begin
<http://www.php.net/manual-lookup.php?lang=en&pattern=begin>();
inbByteBuffer.erase
<http://www.php.net/manual-lookup.php?lang=en&pattern=erase>(it);
}

This loop is wrong,

For a start it should say

for(int i=0; i< messageLength+4 && inbByteBuffer.begin

i.e. < not <=

But even then it is indescribably inefficient because you are copying the
whole vector repeatedly back over itself for each character you delete.

Try this, it's simpler and more efficient.

inbByteBuffer.erase(inbByteBuffer.begin(), inbByteBuffer.begin() +
messageLength + 4);

I don't believe you need to check if the buffer is long enough to delete the
message since you already make that check earlier in the code.

john

[Chris]

Excellent!! thanks a bunch john.

Regards
Chris

John Harrison wrote:

"Chris Thompson" <ch************@tascomi.com> wrote in message
news:10*************@corp.supernews.com...

Hi

I'm writing a p2p client for an existing protocol.

I used a std::vector<char> as a buffer for messages read from the server.
The message length is the first 4 bytes. The message code the second 4.
The total message length is therefore 4 + message length.

A number of messages work fine/as expected but there are consistant
errors occuring. After a period
the message lengths and codes are very large numbers. Another important
(i think) point is that in the while loop, bytes
are only removed from the vector if a whole message exists. Yet on
occasion the messageLength calculated changes
between itterations that do not remove any data? how can this be so?

Is it maybe the case that i should be using std::vector<unsigned char>
to store the data?

Hard to say, but if not you should certainly cast to unsigned char in your
msgLength and msgCode calculations.

// get the message length (first 4)
for(int i=3; i>=0; i--) {
messageLength*=256;
messageLength+= static_cast<unsigned char>(inbByteBuffer[i]);
}

The code i suspect is to blame is below. I can't work out if memory is
getting overwritten/used uninitialised. As i understand it
the erase() call is fine so each complete message is removed correctly.

Any help very welcome as i'm stuck as a big stuck thing at the minute.
Chris

[snip]

// get the iterator
std::vector<char>::iterator it = inbByteBuffer.begin
<http://www.php.net/manual-lookup.php?lang=en&pattern=begin>();

// remove the items from the inbByteBuffer
for(int i=0; i<= messageLength+4 && inbByteBuffer.begin
<http://www.php.net/manual-lookup.php?lang=en&pattern=begin>() !=
inbByteBuffer.end
<http://www.php.net/manual-lookup.php?lang=en&pattern=end>();i++) {
it = inbByteBuffer.begin
<http://www.php.net/manual-lookup.php?lang=en&pattern=begin>();
inbByteBuffer.erase
<http://www.php.net/manual-lookup.php?lang=en&pattern=erase>(it);
}

This loop is wrong,

For a start it should say

for(int i=0; i< messageLength+4 && inbByteBuffer.begin

i.e. < not <=

But even then it is indescribably inefficient because you are copying the
whole vector repeatedly back over itself for each character you delete.

Try this, it's simpler and more efficient.

inbByteBuffer.erase(inbByteBuffer.begin(), inbByteBuffer.begin() +
messageLength + 4);

I don't believe you need to check if the buffer is long enough to delete the
message since you already make that check earlier in the code.

john