This is a rant. I've made it in various forms in various places over the years, and I think it's time to put it somewhere so I can point people at it, and stop repeating myself.
Here's the pebble in my shoe: people keep claiming that strncpy
is a safer alternative to strcpy. It's not. It's a slower function
that is not going to make you significantly safer.
The problem with strcpy is that it's somewhat easy to overflow
the target array:
char src[] = "hello, world";
char dst[5];
strcpy(dst, src); // Oops.
The suggested alternative with strncpy would be this:
strncpy(dst, src, sizeof(dst));
strncpy gets the size of the destination array as a parameter,
and can therefore make sure the string will fit there. Problem solved?
Not so much.
Every time you do any string handling with C's native string
types and functions, you need to make sure you have allocated
enough memory for each string, you need to keep track of
how much memory you've allocated, and you neet to make sure the
zero byte ('NUL', '\0') is put there correctly. This is a lot
of details to keep track of, and it's not surprising things
sometimes go wrong.
If your arrays are allocated statically, which is the simplest,
but least useful case, you need to something like this with
strcpy:
if (strlen(src) + 1 > sizeof(dst))
panic(); // or however you want to handle your errors
strcpy(dst, src);
The +1 after strlen is to make room for the zero byte (NUL,
'\0') that is used in C to indicate the end of a string.
With strncpy, your code is slightly simpler. This
is its attraction.
Problem is, this only works if dst is an array defined within
the scope of the code. If, as is common, you have received dst
as an argument to a function, sizeof(dst) is the size of a
pointer to an element of the array, not the size of the array.
So now you need to track the size explicitly in a second variable:
strncpy(dst, src, dst_size);
Further, strncpy has an unfortunate error handling mode: it merely fills
the destination array and stops. It does not indicate failure in
any way. It does not terminate the target array with a zero
byte. Since all C string processing assume the zero byte is there,
as soon as you try to use dst as a string, you're in trouble.
But luckily you can check for that before you call strncpy:
if (strlen(src) + 1 > dst_size)
panic(); // or whatever
strncpy(dst, src, dst_size);
By this time, the code is equally hard whether you use
strcpy or strncpy. With strncpy you often waste some
extra time, though, filling the unused parts of the target
array with zero bytes: they're almost never necessary.
Alternatively, you can force the zero byte there yourself, silently truncating a string.
strncpy(dst, src, dst_size);
dst[dst_size - 1] = '\0'; // let's hope dst_size > 0
If your string handling is OK with strings being randomly shorter than they should be, possibly you could replace all of them with empty strings, and save yourself a world of pain.
Instead of strncpy, use snprintf. It has sensible
error behavior, and is much more versatile:
n = snprintf(dst, dst_size, "%s", src);
if (n >= dst_size)
panic();
snprintf still isn't very nice to use. It still requires you
to allocate memory yourself, and keep track of the size of each
array manually. It's also slower for small destination buffers
(but faster for large ones).
The real solution is to abandon C's antiquated string handling, and use a more sensible helper library instead. There's a bunch of them around, though none have gained much popularity. I'm not going to name any of them, but just imagine how much nicer it would be to write code like this instead:
Dynstr *src = dynstr_new_from_cstring("hello, world\n");
Dynstr *cat = dynstr_cat_many(src, src, src, src, src, NULL);
Dynstr *dog = dynstr_substr(cat, 15, 5);
Let the library take care of memory allocations and related tracking, and concentrate on real problem solving instead.
We did this in Kannel, back in 1999, and it wiped out all the buffer overflow problems (until a co-worker decided it was "more efficient" to use native C strings instead, promptly introducing a buffer overflow).
All of this string handling is, of course, way nicer in higher level languages. In fact, whenever you need to deal with string handling in C, it is probably a sign that you should consider switching to a higher level language instead.
But whatever you do, stop claiming that strncpy is safe or
solves any real problems strcpy has.
Update: Fixed "It does terminate" to say "It does not terminate". Thanks to Matthew Garrett for pointing that out.
Update2: Fixed example call to snprintf.
Update3: Fixed the test for snprintf example.
Thanks, Redditor axylone.
Also pointed out that snprintf is slower than the rest.
strlcpy, even though it is only marginally better thanstrncpy.strlcpy(dst,src,dst_size)→snprintf(dst_size, dst, "%s", src);Should be the same semantics, and it's completely standard in C99.
You entered the syntax for snprintf correctly in the article but not in your followup comment. Should be snprintf(dst, dst_size, "%s", src) according to the man page.
Thanks for the article, btw!
A fragment of git's strbuf module has escaped into the wild. But presumably a general-purpose version would need a different name to avoid conflicting with the other strbufs out there.
I like the snprintf hint.
Hi, James,
your benchmark is in C++, whereas I was talking about C, but I am not sure that matters here. More importantly, you're testing against a specific, small size of destination buffer. If the destination is larger, snprintf starts winning over strncpy. In my test, 1024 was sufficiently large for that to happen, but the exact cut-off point is going to vary a lot between compilers, optimization levels, and hardware.
But that's what's behind my claim that strncpy is too slow: it always fills the whole destination with stuff, even when it does not need to.
If you don't care about truncation there's also the somewhat ugly pattern of always doing
and thus making sure you never miss the terminating null. In addition to being something of an eyeful to look at, it obviously still suffers from the dst_size == 0 problem.
I suppose the question of how to null-terminate a 0-length object deserves its own discussion
The following code works and is perfectly safe:
strncpy(dst, src, dst_size); if (dst_size == 0 || dst[dst_size-1] != '\0') panic();
Yes, snprintf works equally well and usually there is little reason to use strncpy, except if you do not have snprintf available. But it's not hard to write safe code using strncpy.
Randomly truncated strings are indeed irritating and generally result in bugs. But buffer overflows result in far worse issues. It is better to have a failure mode of randomly truncated strings than it is to have a failure mode that results in an almost certain security vulnerability.
But you are correct about strncpy having its own pitfalls and problems and being far from ideal.