7

I have two strings, str1 and str2. I want the concatenation of them on a space in the heap. I malloc space for them using:

char *concat = (char*) malloc(strlen(str1) + strlen(str2) + 1);

Can I just do:

strcat(concat, str1);
strcat(concat, str2);

And concat will give me the place on the heap with the two strings concatted? I am asking because it seems that strcat would actually add the str1 to the end of the space allocated using malloc. Is this correct? So, then, str1 would appear at position strlen(str1) + strlen(str2) + 1.

The reason that I am asking is that I am using the method above, but I am getting an error in valgrind: Conditional jump or move depends on uninitialised value(s)

Kelp
  • 1,268
  • 5
  • 18
  • 32

3 Answers3

15

What strcat(dest, src) actually does is search for the a null byte starting at dest and going forward, and then write the src string there.

After malloc, the contents of memory are undefined, so your current code could do any number of things, most of them incorrect. If you do concat[0] = 0 before the strcat's, then your code works but will have to search for the length of str1 three times -- once for strlen, again for the first strcat, and last for the second strcat.

Instead though, I recommend using memcpy:

size_t len1 = strlen(str1), len2 = strlen(str2);
char *concat = (char*) malloc(len1 + len2 + 1);

memcpy(concat, str1, len1);
memcpy(concat+len1, str2, len2+1);

This takes advantage of the fact that you know from the start where you want the bytes of both strings to go, and how many there are.

Community
  • 1
  • 1
Walter Mundt
  • 24,753
  • 5
  • 53
  • 61
  • +1 Definitely a very good way to do it. I'd probably use `memmove()` rather than `memcpy()` because, although `memcpy()` is safe in this situation, it is not always safe but `memmove()` is always safe. – Jonathan Leffler Apr 10 '11 at 20:30
  • 4
    Don't use `memmove` here; it's confusing. Any usage of `memmove` should serve to document that you're doing something very different and unusual. If I saw `memmove` here I'd spend a minute or two trying to figure out why the #*$@ someone used it and if anything fishy was going on... – R.. GitHub STOP HELPING ICE Apr 10 '11 at 22:23
6

You want to do a strcpy and then a strcat:

strcpy(concat, str1);
strcat(concat, str2);

strcat relies on there being a null terminator ('\0') to know where to begin. If you just malloc and strcat, it's going to do some nasty things.

And no, neither strcpy nor strcat will do any kind of implicit allocation or reallocation.

Chris Eberle
  • 47,994
  • 12
  • 82
  • 119
5

I would personally do the following:

size_t length = strlen(str1) + strlen(str2) + 1;
char *concat = malloc(sizeof(char) * length);

if(concat == NULL)
{
    // error
}

snprintf(concat, length, "%s%s", str1, str2);
  • +1 for mentioning the error check. `snprintf()` is often a good solution, though it borders on overkill here. – Jonathan Leffler Apr 10 '11 at 20:32
  • 2
    Don't use `strlen` to compute length. Use `length=snprintf(0,0,"%s%s",str1,str2);` Or just use `asprintf` (and be sure to include your own version - it's a trivial wrapper around `snprintf` - for systems which don't include this GNU extension). – R.. GitHub STOP HELPING ICE Apr 10 '11 at 22:18
  • 2
    Justification of R's comment above by himself here (must read to understand the advice): http://stackoverflow.com/a/5615561/340236 – San Oct 22 '13 at 16:47