snprintf for string concatenation

Question

I am using snprintf to concatenate a string to a char array:

char buf[20] = "";
snprintf(buf, sizeof buf, "%s%s", buf, "foo");
printf("%s\n", buf);
snprintf(buf, sizeof buf, "%s%s", buf, " bar");
printf("%s\n", buf);

The problem is the second concatenation to buf instead of adding "bar", replaces "foo" with it. The output is like:

foo
bar

The first %s should keep buf (which in this case holds "foo") there. And the second %s should attach "bar" to it. Right?

What am I doing wrong?

For C++, you should bite the bullet and just use `std::string`. That doesn't help with the C component of your question of course. — paxdiablo, Aug 22 '12 at 01:46
@Scooter: Do NOT go around removing the `c++` tag from questions which use C library functions. Often there are subtle differences in how the same code is treated by a C vs C++ compiler. — Ben Voigt, Aug 22 '12 at 13:12
@BenVoigt I can't directly change any tag, so that means at least two people disagree with you. — Scooter, Aug 22 '12 at 14:39
@Scooter: I don't care how many people agreed with you. It's not hard to find three people on the internet who are wrong. [The expert consensus is that the tag should match the compiler being used.](http://meta.stackexchange.com/a/86338/135695) — Ben Voigt, Aug 22 '12 at 17:23
@BenVoigt Can you point me to the line where the poster says they are using a C++ compiler? And it's not hard to find "experts" who are wrong, such as those who recommended giving mortgages to people they knew couldn't pay them. I would caution you to NOT go around blindly thinking any question about how to use the C standard library routines is something people using C++ want to see. — Scooter, Aug 22 '12 at 22:52
@Scooter: The original poster used the `c++` tag. That's often the only indicator that a C++ compiler is being used, and by removing the tag, you removed important information. — Ben Voigt, Aug 23 '12 at 03:08
The code above is a small portion of a write callback function for curl. And this whole thing is going to be used later on as part of a c project. The reason I put c++ tag there is because I presumed there is a heap of c experts following only c++ tag (14.5k for c and 19.7 for c++). I mean, how could you be a c++ expert without knowing c inside out? — Jermin Bazazian, Aug 23 '12 at 06:56
@Jermin: Don't do that. Now I will remove the `c++` tag, based on your comment that it isn't compiled as C++ after all. — Ben Voigt, Dec 13 '13 at 00:00
*"how could you be a c++ expert without knowing c inside out?"* Quite easily. A C++ expert would be familiar with the C++ standard library, and suggest to use `std::string`'s `+=` operator (or `append` function) for string concatenation. They wouldn't know anything about `snprintf` and certainly wouldn't recommend its use. There are lots of differences between C and C++, *especially* for things like strings and standard algorithms. Tag the question for the answers you want to get! — Cody Gray - on strike, Aug 11 '16 at 18:49

Ben Voigt · Accepted Answer · 2013-12-12T23:57:25.623

35

You're violating the restrict contract on snprintf, which states that no other argument can overlap the buffer.

Copying the input into itself is a waste of effort anyway. snprintf returns the number of characters which formatting would require, so take advantage of this for appending:

char buf[20] = "";
char *cur = buf, * const end = buf + sizeof buf;
cur += snprintf(cur, end-cur, "%s", "foo");
printf("%s\n", buf);
if (cur < end) {
    cur += snprintf(cur, end-cur, "%s", " bar");
}
printf("%s\n", buf);

edited Dec 12 '13 at 23:57

answered Aug 22 '12 at 01:42

Ben Voigt

277,958
43
419
720

1

Is there a document I could refer to for this `restrict` contract? – Jermin Bazazian Aug 22 '12 at 01:46
@Jermin: You mean to see that it applies to `snprintf`, or the meaning of the C99 `restrict` keyword? For the first, see [the man page](http://www.unix.com/man-page/All/3c/snprintf/). For the second, any good recent C book, or [wikipedia](http://en.wikipedia.org/wiki/Restrict). – Ben Voigt Aug 22 '12 at 01:46
Google should make more use of man pages than anything else. I was referring to this [link](http://en.cppreference.com/w/cpp/io/c/fprintf). Thanks. – Jermin Bazazian Aug 22 '12 at 01:51
1

The problem with `+=` is that it is not overflow safe (if there is such a phrase). I am using the above method to expand the size of `buf` if it is going to overflow. – Jermin Bazazian Aug 22 '12 at 01:54
1

@JerminBazazian- `buf` is statically allocated, you can't expand it. Can you explain what you mean? – bta Aug 22 '12 at 01:58
1

@Jermin: `snprintf` cannot overflow. You give it the remaining space in the buffer and it won't write more than that many `char`s. – Ben Voigt Aug 22 '12 at 02:00
1

@BenVoigt: That's exactly why I am using `snprintf`. You are using `+=` to concatenate right? That's what I am referring to as not overflow safe. – Jermin Bazazian Aug 22 '12 at 02:03
@Jermin: No, I am using `+=` to advance the pointer by the number of characters `snprintf` placed into the buffer. Which can never overflow. – Ben Voigt Aug 22 '12 at 02:04
I am impressed by how simple and straightforward this pattern is. Is there any reason not to use it (over strcat and whatnot) when string truncation is acceptable? – kelvin Nov 30 '18 at 23:39
@BenVoigt "I am using += to advance the pointer by the number of characters snprintf placed into the buffer" I believe this is not correct, as `snprintf` returns the number of characters it could have written, not the number it did actually write. The second parameter could then become negative, but is an unsigned `size_t`, so an overflow might indeed happen. – Karsten Koop Sep 29 '22 at 14:29
@KarstenKoop: That behavior is not standard-compliant. https://pubs.opengroup.org/onlinepubs/7908799/xsh/fprintf.html "Upon successful completion, these functions return the number of bytes transmitted excluding the terminating null in the case of `sprintf()` or `snprintf()` or a negative value if an output error was encountered." Which is different from what C says. Conflicting standards, what a mess. – Ben Voigt Sep 30 '22 at 15:05
@KarstenKoop: You will however note that my code does not calculate `end-cur` in the case that would overflow. – Ben Voigt Sep 30 '22 at 15:10

score 4 · Answer 2 · edited Nov 10 '17 at 09:53

Try this:

char buf[20];
snprintf(buf, sizeof buf, "%s", "foo");
printf("%s\n", buf);
int len = strlen(buf);
snprintf(buf+len, (sizeof buf) - len, "%s", " bar");
printf("%s\n", buf);

Output is "foo bar". The first argument to snprintf, a pointer to a char, is where it will start stuffing the characters. It pays no attention to what is in the buffer already. The function strlen does pay attention though. It counts the number of characters before the nul (0) that snprintf put there. So instead of passing buf, pass buf+strlen(buf). You could also use strncat, which would be slightly more efficient.

I see the tag C++ under your question. Look up std::string. Way better.

score 3 · Answer 3 · answered Aug 22 '12 at 02:28

3

While the accepted answer is alright, the better (in my opinion) answer is that concatenating strings is wrong. You should construct the entire output in a single call to snprintf. That's the whole point of using formatted output functions, and it's a lot more efficient and safer than doing pointer arithmetic and multiple calls. For example:

snprintf(buf, sizeof buf, "%s%s%s", str_a, str_b, str_c);

answered Aug 22 '12 at 02:28

R.. GitHub STOP HELPING ICE

208,859
35
376
711

I am using this method for a write-callback in curl and all the strings are not known in advance. I have two strings at a time. The existing buffer and the new one to concatenate with. So, It has to be multiple calls rather than one. – Jermin Bazazian Aug 22 '12 at 02:39
5

This idea is wholly incompatible with loops and conditionals. So I don't think it's actually very helpful, except in very limited circumstances. – Ben Voigt Dec 12 '13 at 23:58
@BenVoigt: I think empirically those "limited circumstances" cover a fairly large portion of real-world usage except in cases where code is written to be gratuitously abstract in place of being simple and efficient. Constructing a string with loops and conditionals when it could be constructed directly with a static format string just makes it harder for someone reading the code to understand what it's doing. Of course there are plenty of exceptions (for example if you're formatting data that's "naturally" an array or complex data structure) but OP's code just had flat consecutive calls. – R.. GitHub STOP HELPING ICE Dec 13 '13 at 00:13
2

@R..: I'm pretty sure the real code does not contain "foo" and "bar". Smells very much like a minimal reproduction. – Ben Voigt Jan 10 '14 at 03:09

bta · Answer 4 · 2012-08-22T02:05:39.047

2

Why not use strncat()? It was designed to do exactly this:

char buf[20] = "";
strncat(buf, "foo", sizeof buf);
printf("%s\n", buf);
strncat(buf, " bar", sizeof buf - strlen(buf));
printf("%s\n", buf);

If your systems supports it you can use strncat_s() instead of strncat, as it has an additional level of overflow protection and avoids the need for calculating the number of bytes remaining in the output buffer.

If you must use snprintf, you will need to create a separate pointer to keep track of the end of the string. This pointer will be the first argument that you pass to snprintf. Your current code always uses buf, which means that it will always print to the beginning of that array. You can either use strlen to find the end of the string after each snprintf call, or you can use the return value of snprintf to increment the pointer.

edited Aug 22 '12 at 02:05

answered Aug 22 '12 at 01:54

bta

43,959
6
69
99

As it is suggested [here](http://stackoverflow.com/questions/308695/c-string-concatenation) `snprintf` is the cleanest way. Not very sure why though. I mean strncat is overflow safe too. – Jermin Bazazian Aug 22 '12 at 01:58
1

@JerminBazazian- `snprintf` is recommended on older (pre-C99) systems where `strncat` is not available and the best you have is `strcat`, which has overflow problems. `strncat` does not have that problem. – bta Aug 22 '12 at 02:01
I assume `snprintf` is used because it provides a whole host of formatting capabilities that `strncat` doesn't. – Ben Voigt Aug 22 '12 at 02:01
1

Not sure, but I think you have an off-by-one error in the buffer size, because according to your link, it doesn't include the NUL byte which is always appended. – Ben Voigt Aug 22 '12 at 02:13
`strncat` was not designed for safety of the output buffer. It was designed for appending non-C-String text data (from fixed-size not-null-terminated fields) to C strings, unsafely. – R.. GitHub STOP HELPING ICE Aug 22 '12 at 02:24
Guys [Here](http://blog.liw.fi/posts/strncpy/) and [here](http://www.joelonsoftware.com/articles/fog0000000319.html) have good points about `strncat` and `snprintf`. Based on what I understand now, `strncat` is more like `ios`:"You put me at stake and you are dead". – Jermin Bazazian Aug 22 '12 at 02:31
@R..- That was the reason why I recommended `strncat_s` if it was available, as it adds overflow protection for the output buffer. `strncat` *can* be used as safely as `snprintf` (you must keep track of how much buffer space is remaining in both cases), and it should have less overhead since there's no format string to parse. – bta Aug 22 '12 at 16:31
Note that even if the target buffer initially contains an empty string, as shown, if you call `strncat()` with `sizeof(buf)` and the source string is too long (must be truncated), you immediately have a buffer overflow — the very thing you were trying to guard against. This is a primary reason not to use `strncat()` — it has a very counter-intuitive interface. The other reason not to use `strncat()` is that if you know enough to be able to use it safely, you could use other functions instead (notably `memmove()` or `memcpy()`), because they would not need to skip over the existing data. – Jonathan Leffler May 21 '19 at 18:04

snprintf for string concatenation

4 Answers4

Linked

Related