views:

8817

answers:

9

I'm working in C and I have to concatenate a few things.

Right now I have this:

message = strcat("TEXT " + var);

message2 = strcat(strcat("TEXT ", foo), strcat(" TEXT ", bar));

Now if you have experience in C I'm sure you realize that this gives you a Segmentation Fault when you try to run it. So how do i work around that?

+7  A: 

In C, strings are just char arrays. So you can't add to them directly.

Here is an example from cplusplus.com:

char str[80];
strcpy (str,"these ");
strcat (str,"strings ");
strcat (str,"are ");
strcat (str,"concatenated.");

So in your example, you would want:

char *foo = "foo";
char *bar = "bar";
char str[80];
strcpy (str, "TEXT ");
strcat (str, foo);
strcat (str, bar);

The return value of strcat can just be ignored, it returns the same pointer as was passed in for the first argument. You don't need this value. The return value is simply for convenience, so it allows you to chain the calls into one line of code.

For the first parameter, you need to have the destination buffer itself. The destination buffer must be a char array buffer. Example char buffer[1024];

Be very careful that the first parameter has enough space for what you're trying to copy in. If available to you, it is safer to use functions like: strcpy_s and strcat_s which explicitly specify the size of the destination buffer.

You can never use a string literal as a buffer, you must always use your own buffer.

Brian R. Bondy
Would you put "Be very careful that..." in bold, please? This can't be stressed enough. Misuse of strcat, strcpy, and sprintf are the heart of unstable/insecure software.
plinth
"static string" and "allocate your own buffer" may be misleading as the former could be interpreted to mean "static string buffer" and the latter to indicate dynamical allocation. +1 if you change to "You can never use a string literal as a buffer, you must always use your own buffer" or similar.
Robert Gamble
Warning: As written, this code will leave a giant, gaping hole in your code for buffer overflow exploits.
Brian
There is no buffer overflow exploit possible in the above example. And yes I agree in general I wouldn't use the above example for undetermined string lengths of foo and bar.
Brian R. Bondy
Robert Gamble: Thanks for the suggestion, changed it so it is more clear.
Brian R. Bondy
plinth: Thanks for the suggestion, I bolded the line
Brian R. Bondy
Brian: Thanks, +1 as promised :)
Robert Gamble
Don't use strcat! sprintf is much better!
psihodelia
@psihodelia: Also don't forget that spoons are much better than forks! so be sure to always use a spoon!
Brian R. Bondy
Don't use `strcat` multiple times! `strcat` has to check all the previous bytes (searching for `'\0'`) you already concatenated. This is useless processing.
dolmen
+1  A: 

Try this. It achieves the same result as your original:

char message[1000];
strcpy (message, "TEXT ");
strcat (message, var);

char message2[1000];
strcpy (message2, "TEXT ");
strcat (message2, foo);
strcat (message2, " TEXT ");
strcat (message2, bar);
paxdiablo
What happens if var/foo/bar has more than 1000 characters? >:)
Geo
Then you will get a buffer overflow, which you can add code to check for beforehand (say, with strlen). But the purpose of a code snippet is to show how something works without polluting it with too much extra code. Otherwise I'd be checking lengths, whether var/foo/bar was null, etc.
paxdiablo
+4  A: 

The first argument of strcat() needs to be able to hold enough space for the concatenated string. So allocate a buffer with enough space to receive the result.

char bigEnough[64] = "";

strcat(bigEnough, "TEXT");
strcat(bigEnough, foo);

/* and so on */

strcat() will concatenate the second argument with the first argument, and store the result in the first argument, the returned char* is simply this first argument, and only for your convenience.

You do not get a newly allocated string with the first and second argument concatenated, which I'd guess you expected based on your code.

Pieter
+1  A: 

You are trying to copy a string into an address that is statically allocated. You need to cat into a buffer.

Sepcifically: ...snip... destination Pointer to the destination array, which should contain a C string, and be large enough to contain the concatenated resulting string. ...snip...

http://www.cplusplus.com/reference/clibrary/cstring/strcat.html

There's an example here as well.

Todd
+13  A: 

Avoid using strcat. The cleanest and, most importantly, the safest way is to use snprintf:

char buf[256];
snprintf(buf, sizeof(buf), "%s%s%s%s", str1, str2, str3, str4);
Alex B
... but without the needless and confusing parenthesis to the sizeof operator. Those are only needed when you want the size of an actual type, not of an object. Pet peeve alert, sorry.
unwind
This is arguably the best way to find out the size of the buffer, unless you are given a pointer. If you have a pointer, you will be passing the length of the buffer it points to with it.
Alex B
Checkers, he was talking about the parentheses around "buf" of the sizeof argument. they are not required if the argument is an expression. But i don't understand why you are downvoted. i think your answer is the best of all, even though it is c99. (maybe because of that they disagree! lamers!) +1
Johannes Schaub - litb
Oh, reading comprehension -1. But anyway, I stand by my answer.
Alex B
sizeof() only works here for char buf[...]. NOT for char * buf = malloc(...). There aren't many differences between arrays and pointers, but this is one of them!
Mr.Ree
Yes, hence my comment about dynamically allocated buffers.
Alex B
Also, he is trying to perform concatenation. Concatenating using `snprintf()` is a BIG no no.
Leonardo Herrera
@Leonardo, can you tell use why?
quinmars
Well, there is some risk that you add another, say, "str5" and forget an additional "%s" in the format string - or the other way around. Will only crash on you (if you are lucky) on runtime. Besides it seems a bit overkill given the _safe_ alternatives of "strcat".
Christian.K
A: 

Hi, if you have experience in C you will notice that Strings are only char arrays where the last character is a null character.

Now that is quite inconvenient as you have to find the last character in order to append something. strcat will do that for you. So strcat searches through the first argument for a null character. Then it will replace this with the second argument's content (until that ends in a null).

Now let's go through your code:

message = strcat("TEXT " + var);

Here you are adding something to the pointer to the text "TEXT" (the type of "TEXT" is const char*. A pointer.). That will usually not work. Also modifying the "TEXT" array will not work as it is usually placed in a constant segment.

message2 = strcat(strcat("TEXT ", foo), strcat(" TEXT ", bar));

That might work better, except that you are again trying to modify static texts. strcat is not allocating new memory for the result.

I would propose to do something like this instead:

sprintf( message2, "TEXT %s TEXT %s", foo, bar );

Read the documentation of sprintf to check for it's options.

And now an important point. Ensure that the buffer has enough space to hold the text AND the null character. There are a couple of functions that can help you e.g. strncat and special versions of printf that allocate the buffer for you. Not ensuring the buffer size will lead to memory corruption and remotely exploitable bugs.

Ralf
+1  A: 

Folks, use strncpy(), strncat(), or snprintf().
Exceeding your buffer space will trash whatever else follows in memory!
(And remember to allow space for the trailing NULL '\0' character!)

Mr.Ree
Not only should you remember to allow space for the NULL character, you need to remember to *add* the NULL character. strncpy and strncat don't do that for you.
Graeme Perrow
Uh? strncpy() and strncat() sure add the terminating character. In fact, they add too many. At least as long as there's space left in the buffer, which is a huge trap with these calls. Not recommended.
unwind
@unwind, I think the point of Graeme is that if the buffer is too small, strncpy or strncat will _not_ add the terminating '\0'.
quinmars
if n < strlen()+1, no null is added. if n==strlen()+1, one null is added. if n>strlen()+1 many nulls are added. It's called PROGRAMMING.
Mr.Ree
Even better, prefer OpenBSD's strlcpy, strlcat (easy to port into your own project).
Alex B
Right, I prefer strl*, too. But in this case I think your snprintf proposal is the best way.
quinmars
snprintf is good, strncpy/strncat is the worst possible recommendation, strlcpy/strlcat is much better.
Robert Gamble
A: 

Do not forget to initialize the output buffer. The first argument to strcat must be a null terminated string with enough extra space allocated for the resulting string:

char out[1024] = ""; // must be initialized
strcat( out, null_terminated_string ); 
// null_terminated_string has less than 1023 chars
David Rodríguez - dribeas
+1  A: 

Also malloc and realloc are useful if you don't know ahead of time how many strings are being concatenated.

#include <stdio.h>
#include <string.h>

void example(const char *header, const char **words, size_t num_words)
{
    size_t message_len = strlen(header) + 1; /* + 1 for terminating NULL */
    char *message = (char*) malloc(message_len);
    strncat(message, header, message_len);

    for(int i = 0; i < num_words; ++i)
    {
       message_len += 1 + strlen(words[i]); /* 1 + for separator ';' */
       message = (char*) realloc(message, message_len);
       strncat(strncat(message, ";", message_len), words[i], message_len);
    }

    puts(message);

    free(message);
}
Reed Hedges