How can I use strncat without buffer overflow concerns?

I have a buffer, I am doing l开发者_如何学Cot of strncat. I want to make sure I never overflow the buffer size.

char buff[64];

strcpy(buff, "String 1");

strncat(buff, "String 2", sizeof(buff));

strncat(buff, "String 3", sizeof(buff));

Instead of sizeof(buff), I want to say something buff - xxx. I want to make sure I never override the buffer

Take into consideration the size of the existing string and the null terminator

#define BUFFER_SIZE 64
char buff[BUFFER_SIZE];

//Use strncpy
strncpy(buff, "String 1", BUFFER_SIZE - 1);
buff[BUFFER_SIZE - 1] = '\0';

strncat(buff, "String 2", BUFFER_SIZE - strlen(buff) - 1);

strncat(buff, "String 3", BUFFER_SIZE - strlen(buff) - 1);

Why not use snprintf? Unlike strncat it expects the size of the buffer, but more importantly, there's no hidden O(n).

Strcat needs to find the null-terminator on each string it concatenates, and each time run through the whole buffer to find the end. Each time the string gets longer, strcat slows down. Sprintf, on the other hand can keep track of the end. you'll find that

snprintf(buf, sizeof buf, "%s%s%s", "String1", "String2", "String3");

Is frequently a faster, and more readable soluton.

The way you use the strncat function in your orignal code would actually be appropriate for another function: strlcat (note l instead of n). The strlcat function is not standard, yet it is a popular implementation-provided replacement for strncat. strlcat expects the total size of the entire destination buffer as its last argument.

Meanwhile, strncat expects the size of the remaining unused portion of the target buffer as its third argument. For this reason, your original code is incorrect.

I would suggest that instead of doing that horrible abuse of strncpy and making explicit rescans with those strlen calls (both issues present in Joe's answer), you either use an implementation-provided strlcat or implement one yourself (if your implementation provides no strlcat).

http://en.wikipedia.org/wiki/Strlcpy

This is the best way to do it. sizeof() just gives you size of the pointer to the data if you don't allocate it locally (you did allocate locally in this case but better to do it this way and it will work if the code is re-factored).

#define MAXBUFFSIZE 64

char buff[MAXBUFFSIZE];

buff[0] = 0;  // or some string

strncat(buff, "String x",MAXBUFFSIZE - strlen(buff) - 1);

Hogan has answered the question sufficently; however, if you are worried about buffer overflows in strcat(...) you should equally be worried about buffer overflows in all the other string functions.

Use strnlen(...) and strncpy(...) to really make sure you stay within your buffer. If you don't have a strnlen(...) function, write it.

I'd use memccpy instead of strncat in this case - it's safer and much faster. (It's also faster than the approach with snprintf mentioned by Dave):

/**
 * Returns the number of bytes copied (not including terminating '\0').
 * Always terminates @buf with '\0'.
 */ 
int add_strings(char *buf, int len)
{
    char *p = buf;

    if (len <= 0)
        return 0;

    p[len - 1] = '\0'; /* always terminate */

    p = memccpy(buf, "String 1", '\0', len - 1);
    if (p == NULL)
        return len - 1;

    p = memccpy(p - 1, "String 2", '\0', len - 1 - (p - buf));
    if (p == NULL)
        return len - 1;

    p = memccpy(p - 1, "String 3", '\0', len - 1 - (p - buf));

    return (p == NULL ? len : p - buf) - 1;
}