String is not null terminated error

Question

I'm having a string is not null terminated error, though I'm not entirely sure why. The usage of std::string in the second part of the code is one of my attempt to fix this problem, although it still doesn't work.

My initial codes was just using the buffer and copy everything into client_id[]. The error than occurred. If the error is correct, that means I've got either client_ id OR theBuffer does not have a null terminator. I'm pretty sure client_id is fine, since I can see it in debug mode. Strange thing is buffer also has a null terminator. No idea what is wrong.

char * next_token1 = NULL;
char * theWholeMessage = &(inStream[3]);
theTarget = strtok_s(theWholeMessage, " ",&next_token1);
sendTalkPackets(next_token1, sizeof(next_token1) + 1, id_clientUse, (unsigned int)std::stoi(theTarget));

Inside sendTalkPackets is. I'm getting a string is not null terminated at the last line.

void ServerGame::sendTalkPackets(char * buffer, unsigned int buffersize, unsigned int theSender, unsigned int theReceiver)
{
std::string theMessage(buffer);
theMessage += "0";

const unsigned int packet_size = sizeof(Packet);
char packet_data[packet_size];
Packet packet;
packet.packet_type = TALK;

char client_id[MAX_MESSAGE_SIZE];


char theBuffer[MAX_MESSAGE_SIZE];
strcpy_s(theBuffer, theMessage.c_str());
//Quick hot fix for error "string not null terminated"

const char * test = theMessage.c_str();
sprintf_s(client_id, "User %s whispered: ", Usernames.find(theSender)->second.c_str());
printf("This is it %s ", buffer);
strcat_s(client_id, buffersize , theBuffer);

Answer 1

Methinks that problem lies in this line:

sendTalkPackets(next_token1, sizeof(next_token1) + 1, id_clientUse, (unsigned int)std::stoi(theTarget));

sizeof(next_token1)+1 will always gives 5 (on 32 bit platform) because it return size of pointer not size of char array.

Answer 2

You can't just do

std::string theMessage(buffer);
theMessage += "0";

This fails on two fronts:

• The std::string constructor doesn't know where buffer ends, if buffer is not 0-terminated. So theMessage will potentially be garbage and include random stuff until some zero byte was found in the memory beyond the buffer.
• Appending string "0" to theMessage doesn't help. What you want is to put a zero byte somewhere, not value 0x30 (which is the ascii code for displaying a zero).

The right way to approach this, is to poke a literal zero byte buffersize slots beyond the start of the buffer. You can't do that in buffer itself, because buffer may not be large enough to accomodate that extra zero byte. A possibility is:

char *newbuffer = malloc(buffersize + 1);
strncpy(newbuffer, buffer, buffersize);
newbuffer[buffersize] = 0; // literal zero value

Or you can construct a std::string, whichever you prefer.

Answer 3

One thing which could be causing this (or other problems): As buffersize, you pass sizeof(next_token1) + 1. next_token1 is a pointer, which will have a constant size of (typically) 4 or 8. You almost certainly want strlen(next_token1) + 1. (Or maybe without the + 1; conventions for passing sizes like this generally only include the '\0' if it is an output buffer. There are a couple of other places where you're using sizeof, which may have similar problems.

But it would probably be better to redo the whole logic to use std::string everywhere, rather than all of these C routines. No worries about buffer sizes and '\0' terminators. (For protocol buffers, I've also found std::vector<char> or std::vector<unsigned char> quite useful. This was before the memory in std::string was guaranteed to be contiguous, but even today, it seems to correspond more closely to the abstraction I'm dealing with.)