Memcpy, string and terminator

Question

I have to write a function that fills a char* buffer for an assigned length with the content of a string. If the string is too long, I just have to cut it. The buffer is not allocated by me but by the user of my function. I tried something like this:

int writebuff(char* buffer, int length){
    string text="123456789012345";
    memcpy(buffer, text.c_str(),length);
    //buffer[length]='\0';
    return 1;
}


int main(){
    char* buffer = new char[10];
    writebuff(buffer,10);
    cout << "After: "<<buffer<<endl;
}

my question is about the terminator: should it be there or not? This function is used in a much wider code and sometimes it seems I get problems with strange characters when the string needs to be cut.

Any hints on the correct procedure to follow?

Answer 1

A C-style string must be terminated with a zero character '\0'.

In addition you have another problem with your code - it may try to copy from beyond the end of your source string. This is classic undefined behavior. It may look like it works, until the one time that the string is allocated at the end of a heap memory block and the copy goes off into a protected area of memory and fails spectacularly. You should copy only until the minimum of the length of the buffer or the length of the string.

P.S. For completeness here's a good version of your function. Thanks to Naveen for pointing out the off-by-one error in your terminating null. I've taken the liberty of using your return value to indicate the length of the returned string, or the number of characters required if the length passed in was <= 0.

int writebuff(char* buffer, int length)
{
    string text="123456789012345";
    if (length <= 0)
        return text.size();
    if (text.size() < length)
    {
        memcpy(buffer, text.c_str(), text.size()+1);
        return text.size();
    }
    memcpy(buffer, text.c_str(), length-1);
    buffer[length-1]='\0';
    return length-1;
}

Answer 2

If you want to treat the buffer as a string you should NULL terminate it. For this you need to copy length-1 characters using memcpy and set the length-1 character as \0.