std::vector<char> troubles

Collapse
This topic is closed.
X
X
Collapse
 
  • Page of 1
  • Time
  • Show
Clear All
new posts
Previous template Next
  • Chris Thompson
    #1

    std::vector<char> troubles

    Hi

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

    I used a std::vector<cha r> 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<uns igned 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<cha r>
    | // if we have got the message length and code bytes
    while (inbByteBuffer. size
    <http://www.php.net/manual-lookup.php?lang =en&pattern=siz e>() >= 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.s ize
    <http://www.php.net/manual-lookup.php?lang =en&pattern=siz e>()) {
    return false;
    }

    // create the bytes specific to this message
    std::vector<cha r> 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=pus h_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=bui ldMessage>(data );

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

    // get the iterator
    std::vector<cha r>::iterator it = inbByteBuffer.b egin
    <http://www.php.net/manual-lookup.php?lang =en&pattern=beg in>();

    // remove the items from the inbByteBuffer
    for(int i=0; i<= messageLength+4 && inbByteBuffer.b egin
    <http://www.php.net/manual-lookup.php?lang =en&pattern=beg in>() !=
    inbByteBuffer.e nd
    <http://www.php.net/manual-lookup.php?lang =en&pattern=end >();i++) {
    it = inbByteBuffer.b egin
    <http://www.php.net/manual-lookup.php?lang =en&pattern=beg in>();
    inbByteBuffer.e rase
    <http://www.php.net/manual-lookup.php?lang =en&pattern=era se>(it);
    }
    }

    |
    Tags: None
    • John Harrison
      #2
      Re: std::vector&lt; char&gt; troubles


      "Chris Thompson" <chris.thompson @tascomi.com> wrote in message
      news:104cpddn7a 9ki93@corp.supe rnews.com...[color=blue]
      > Hi
      >
      > I'm writing a p2p client for an existing protocol.
      >
      > I used a std::vector<cha r> 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<uns igned char>
      > to store the data?[/color]

      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<uns igned char>(inbByteBu ffer[i]);
      }
      [color=blue]
      >
      > 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
      >[/color]

      [snip]
      [color=blue]
      >
      > // get the iterator
      > std::vector<cha r>::iterator it = inbByteBuffer.b egin
      > <http://www.php.net/manual-lookup.php?lang =en&pattern=beg in>();
      >
      > // remove the items from the inbByteBuffer
      > for(int i=0; i<= messageLength+4 && inbByteBuffer.b egin
      > <http://www.php.net/manual-lookup.php?lang =en&pattern=beg in>() !=
      > inbByteBuffer.e nd
      > <http://www.php.net/manual-lookup.php?lang =en&pattern=end >();i++) {
      > it = inbByteBuffer.b egin
      > <http://www.php.net/manual-lookup.php?lang =en&pattern=beg in>();
      > inbByteBuffer.e rase
      > <http://www.php.net/manual-lookup.php?lang =en&pattern=era se>(it);
      > }[/color]

      This loop is wrong,

      For a start it should say

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

      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.e rase(inbByteBuf fer.begin(), inbByteBuffer.b egin() +
      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


        Comment

        • Chris
          #3
          Re: std::vector&lt; char&gt; troubles

          Excellent!! thanks a bunch john.

          Regards
          Chris

          John Harrison wrote:[color=blue]
          > "Chris Thompson" <chris.thompson @tascomi.com> wrote in message
          > news:104cpddn7a 9ki93@corp.supe rnews.com...
          >[color=green]
          >>Hi
          >>
          >>I'm writing a p2p client for an existing protocol.
          >>
          >>I used a std::vector<cha r> 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<uns igned char>
          >>to store the data?[/color]
          >
          >
          > 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<uns igned char>(inbByteBu ffer[i]);
          > }
          >
          >[color=green]
          >>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
          >>[/color]
          >
          >
          > [snip]
          >
          >[color=green]
          >> // get the iterator
          >> std::vector<cha r>::iterator it = inbByteBuffer.b egin
          >><http://www.php.net/manual-lookup.php?lang =en&pattern=beg in>();
          >>
          >> // remove the items from the inbByteBuffer
          >> for(int i=0; i<= messageLength+4 && inbByteBuffer.b egin
          >><http://www.php.net/manual-lookup.php?lang =en&pattern=beg in>() !=
          >>inbByteBuffer .end
          >><http://www.php.net/manual-lookup.php?lang =en&pattern=end >();i++) {
          >> it = inbByteBuffer.b egin
          >><http://www.php.net/manual-lookup.php?lang =en&pattern=beg in>();
          >> inbByteBuffer.e rase
          >><http://www.php.net/manual-lookup.php?lang =en&pattern=era se>(it);
          >> }[/color]
          >
          >
          > This loop is wrong,
          >
          > For a start it should say
          >
          > for(int i=0; i< messageLength+4 && inbByteBuffer.b egin
          >
          > 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.e rase(inbByteBuf fer.begin(), inbByteBuffer.b egin() +
          > 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
          >
          >[/color]

            Comment

            Previous template Next
            Working...