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;
}
精彩评论