-1

I know that I have to use strcpy / strncpy to assign a new string value to an existing char array. Recently I saw a lot of code like this

char arr[128] = "\0";
sprintf(arr, "Hello World"); // only string constants no variable input
// or
sprintf(arr, "%s", "Hello World");

Both variants give the same result. What is the advantage of the latter variant?

Michael Schäfer
  • 177
  • 1
  • 7
  • 2 Q's here: "best method ....." and what is the diff amongst 2 approaches. Which question is more important here? – chux - Reinstate Monica Mar 09 '20 at 21:00
  • 2
    Pretty sure that code was written by a newbie, whose example you should not follow. The giveaway is the string `"\0"` which demonstrates that the author doesn't understand how strings work in C. – user3386109 Mar 09 '20 at 21:14

2 Answers2

4

It depends on whether the string to be copied is a literal, as shown, or can vary.

The best technique for the array shown would be:

char arr[128] = "Hello World";

If you're in charge of the string and it contains no % symbols, then there's not much difference between the two sprintf() calls. Strictly, the first uses the string as the format and copies the characters directly, while the second notes it has %s as the format and copies the characters from the extra argument directly — it's immeasurably slower. There's a case for:

snprintf(arr, sizeof(arr), "%s", "Hello World");

which ensures no buffer overflow even if "Hello World" becomes a much longer diatribe.

If you're not in charge of the string, then using snprintf() as shown becomes important as even if the string contains % symbols, it is simply copied and there's no overflow. You have to check the return value to establish whether any data was truncated.

Using strcpy() is reasonable if you know how long the string is and that there's space to hold it. Using strncpy() is fraught — it null pads to full length if the source is shorter than the target, and doesn't null terminate if the source is too long for the target.

If you've established the length of the string is short enough, using memmove() or memcpy() is reasonable too. If the string is too long, you have to choose an error handling strategy — truncation or error.

If the trailing (unused) space in the target array must be null bytes (for security reasons, to ensure there's no leftover password hidden in it), then using strncpy() may be sensible — but beware of ensuring null termination if the source is too long. In most cases, the initializer for the array is not really needed.

The compiler may be able to optimize the simple cases.

Jonathan Leffler
  • 730,956
  • 141
  • 904
  • 1,278
  • `memcpy(dst, src, min(sizeof(dst), sizeof(src));` seems to be the best option to me (quite obvious what it's doing). You may need to take care of nul termination though. – Aykhan Hagverdili Mar 09 '20 at 20:52
  • 2
    That's feasible if `sizeof(src)` is sensibly evaluable — it depends on where in the world the "Hello" is coming from — and you have to do work to ensure null termination, as you mention. More context is needed to give a more concise answer. – Jonathan Leffler Mar 09 '20 at 20:56
  • what do you mean by "immeasurably slower"? That it's slower but you can't measure how much? That it's enormously slower? I would think it would be slightly faster, particularly if the string is long, but I haven't actually benchmarked `s(n)printf` implementations so that's just my prejudice. – rici Mar 09 '20 at 21:46
  • The difference in performance is likely to be so small that it cannot be measured, @rici. The difference is roughly the time taken to scan `%s` and pop an argument with `va_arg`. Not quite zero, but close to it and probably not a factor in the overall performance of any normal program. And the security from using `%s` is probably worth the minuscule performance loss. – Jonathan Leffler Mar 09 '20 at 21:51
  • 1
    @JonathanLeffler: I did a quick bench with gcc and as expected, with very large arguments the `%s` version wins (measurable but small difference). My hypothesis was based on the fact that if you put the string in the format, sprintf has to compare every character with percent, whereas if you make it a string argument, it only has to check for NUL. – rici Mar 09 '20 at 22:10
  • Anyway, I agree that the safety of `%s` is more important, but I'd always worked on the basis that `%s` is both safer and (a tiny amount) faster. I've sold it to colleagues on that basis, too. – rici Mar 09 '20 at 22:11
  • @rici — interesting and makes sense. I hadn’t thought of it in those terms. So the safety solution is also a performance solution. It’s not often those work together. – Jonathan Leffler Mar 09 '20 at 22:13
3

The first version won't work if the string contains any % characters, because sprintf() will treat them as formatting operators that need to be filled in using additional arguments.. This isn't a problem with a fixed string like Hello World, but if you're getting the string dynamically it could cause undefined behavior because there won't be any arguments to match the formatting operators. This can potentially cause security exploits.

If you're not actually doing any formatting, a better way is to just use strcpy():

strcpy(arr, "Hello World");

Also, when initiallizing the string it's not necessary to put an explicit \0 in the string. A string literal always ends with a null byte. So you can initialize it as:

char arr[128] = "";

And if you're immediately overwriting the variable with sprintf() or strcpy(), you don't need to initialize it in the first place.

Barmar
  • 741,623
  • 53
  • 500
  • 612
  • 3
    Maybe worth noting that not having a format string is in some cases a security flaw. If the string to be printed is received from the input then when used as format string can be exploited to either access or even modify (with `%n`) memory it should not. – Eugene Sh. Mar 09 '20 at 20:40