CODEWITHSUNDEEP
cstring
Jermin Bazazian
Jermin Bazazian

Reputation: 1970

snprintf for string concatenation

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?

Upvotes: 25

Views: 44404

Answers (4)

Jive Dadson
Jive Dadson

Reputation: 17036

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.

Upvotes: 4

Ben Voigt
Ben Voigt

Reputation: 283733

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);

Upvotes: 35

R.. GitHub STOP HELPING ICE
R.. GitHub STOP HELPING ICE

Reputation: 215387

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);

Upvotes: 5

bta
bta

Reputation: 45057

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.

Upvotes: 2

Related Questions