Error with parsing string and trying to find '\0' character

Question

I'm trying to get one side to send an error message to client, but client isn't able to parse it correctly.

My error is >>>>> in my parseString function, it lets index = 0 and therefore I get an out of range for my 'substr' call.

Server Side:::

   #define ERRBUFSIZE 51

    string error = "Error - Already Registered: ";
          error.append(name);
          char err[ERRBUFSIZE];
          err[0] = 0;
          std::copy(error.begin(), error.end(), err+1);

          err[error.size() + 2] = '\0';

          if (send(sock, &err, ERRBUFSIZE, 0) < 0)
          {
          DieWithError("send() failed");
          }

Client side ( who is receiving )::

   char msgBuffer[ERRBUFSIZE];
        int bytesRcvd = 0;
        int totalRcvd = 0;
        while(totalRcvd != ERRBUFSIZE)
        { 
         // Get Message from Server
         if ((bytesRcvd = recv(sock, msgBuffer, ERRBUFSIZE, 0)) < 0)
         DieWithError("recv() failed or connection closed prematurely");

         totalRcvd += bytesRcvd; 
        }

        cout << "Bytes received: " << totalRcvd << endl;
        msgBuffer[totalRcvd] = '\0';

        string rcvMsg( msgBuffer , msgBuffer + totalRcvd);
        cout << rcvMsg << endl;
        rcvMsg = parseString(rcvMsg);

        return rcvMsg;

where....

   string TCPClient::parseString(string message)
    {
        cout << "parsing new string" << endl;
        int index = message.find('\0');
        string result = message.substr(0, index);
        cout << "THE RESULT :: " << result << endl;
        return result;
    }

Answer 1

Why not change your parseString to the following:

string TCPClient::parseString(const string& message, int strLength )
{
    cout << "parsing new string" << endl;
    string result = message.substr( 0, strLength );
    cout << "THE RESULT :: " << result << endl;
    return result;
}

And then change the calling code to:

rcvMsg = parseString(rcvMsg, totalRcvd );

Failing that you could even use the string's "size()" function.

Edit: Its also worth noting that find will advance through the string until the '\0' character is found but WON'T actually look for it. It seems a bit of an odd plan to put a string into a string object (Which stores length) and then try and calculate length yourself through a, slow, iterative process.

Edit2: If you just want to do it with a character array you can do a manual search as follows.

int index = 0;
while( *charString++ != '\0' ) index++;

Its that simple :)

Answer 2

From the (very-difficult-to-read-)code, I think you might be running into "indexing hell".

Check that totalRecvd actually is the index where you want to write that \0 and that when you create your rcvMsg you're adding the \0 index to it (msgBuffer + totalRcvd looks dogdy when you've put your end-of-string marker on the totalRcvd byte, usually you get a string between (start, end-1) indexes you write).

Also - searching for the \0 makes no sense as that is just a marker.

Also (2) - if totalRcvd == ERRBUFSIZE you can't access msgBuffer [totalRcvd], you only have access to the indexes in range (0, ERRBUFSIZE -1)

Answer 3

Use std::vector instead of std::string when you want to manage buffers. '\0' and std::basic_string doesn't combine well.

Answer 4

First, the protocol is nasty. You're sending fixed-size blocks when you want to send a nul-terminated string ... it would be simpler just to put the string length at the front of the message.

Second, you're not reading correctly from the socket, it just happens to work so long as you don't have any other messages in the pipeline. If your while loop was ever really exercised, you'd overwrite the first fragment with any subsequent reads.

Anyway, the proximate cause of the bug is that you prepend a nul terminator (separator?) to the string. Doesn't that explain why you always find your nul at index 0? If you can't see it, try writing out the contents of the buffer as sent.

#define ERRBUFSIZE 51

void server(string const &name)
{
    string error = "Error - Already Registered: ";
    error.append(name);
    char err[ERRBUFSIZE];
    // you sure you want a NUL terminator at index 0??
    err[0] = 0;
    // can replace both lines with strcpy(err, error.c_str())
    std::copy(error.begin(), error.end(), err+1);
    err[error.size() + 2] = '\0';

    if (send(sock, &err, ERRBUFSIZE, 0) < 0)
        DieWithError("send() failed");
}

string parseString(string const &message) // no need to copy
{
    // ... this is redundant anyway, see below
}

string client()
{
    char msgBuffer[ERRBUFSIZE];
    int bytesRcvd = 0;
    int totalRcvd = 0;
    while(totalRcvd != ERRBUFSIZE)
    {       
        // Get Message from Server
        // ... recv(sock, msgBuffer+totalRcvd, ERRBUFSIZE-totalRcvd) ...
        if ((bytesRcvd = recv(sock, msgBuffer, ERRBUFSIZE, 0)) < 0)
            DieWithError("recv() failed or connection closed prematurely");

        totalRcvd += bytesRcvd; 
    }

    cout << "Bytes received: " << totalRcvd << endl;
    //sizeof(msgBuffer)==ERRBUFSIZE==totalRcvd, so this overflows
    msgBuffer[totalRcvd] = '\0';

    // assuming you skip the leading \0, msgBuffer is already nul-terminated
    // so use string rcvMsg(msgBuffer) and skip the parseString/substr
    string rcvMsg( msgBuffer , msgBuffer + totalRcvd);
    cout << rcvMsg << endl;
    // rcvMsg = parseString(rcvMsg);

    return rcvMsg;
}