What is a safe way to join strings in C?

Question

I need to construct a path to a file from two strings. I could use this (not tested, though):

/* DON'T USE THIS CODE! */
/* cmp means component */
char *path_cmp1 = "/Users/john/";
char *path_cmp2 = "foo/bar.txt";
unsigned len = strlen(path_cmp1);
char *path = path_cmp1;
for (int i = 0; i < strlen(path_cmp2); i++) {
  path[len + i] = path_cmp2[i];
}

but this could lead to memory corruption I guess. Is there a better way to do this, or is there a function for this in the standard library?

Solution 1

#include <stdlib.h>
#include <string.h>

char *join(const char* s1, const char* s2)
{
    char* result = malloc(strlen(s1) + strlen(s2) + 1);

    if (result) // thanks @pmg
    {
        strcpy(result, s1);
        strcat(result, s2);
    }

    return result;
}

This is simple enough to be written in place, especially when you have multiple strings to concatenate.

Note that these functions return their destination argument, so you can write

char* result = malloc(strlen(s1) + strlen(s2) + 1);
assert(result);
strcat(strcpy(result, s1), s2);

but this is less readable.

Solution 2

#include <stdio.h> 

char *a = "hello ";
char *b = "goodbye";
char *joined;

asprintf(&joined, "%s%s", a, b)

Solution 3

There are several problems in this code: 1 - calling strlen on the for loop is a bad idea, it will calculate the string length every iteration, so it is better to call it once before the loop and keep the result in a variable.

2 - The same strlen problem applies to strlen(path_cmp1) inside the loop, call it before the loop and increments its size.

In the end, it is better to simple copy both strings and store those on a dynamic allocated string, like:

char *join_strings(const char* s1, const char* s2)
{
    size_t lens1 = strlen(s1);
    size_t lens2 = strlen(s2);

    //plus 1 for \0
    char *result = malloc(lens1 + lens2 + 1);

    if(result)
    {
        memcpy(result, s1, lens1);
        memcpy(result+lens1, s2, lens2+1);
    }

    //do not forget to call free when do not need it anymore
    return result;
}

Solution 4

There are strcat and strncat for that.

Solution 5

char *path_cmp1 = "/Users/john/";
char *path_cmp2 = "foo/bar.txt";

int firstLength = strlen(path_cmp1);
int secondLength = strlen(path_cmp2);
char *both = malloc(firstLength+secondLength+1);
memcpy(both, path_cmp1, firstLength);
memcpy(both+firstLength, path_cmp2, secondLength+1);
       // this +1 copyes the second string's null-terminator too.

View more solutions

Comments

[pmg]

+1 ... but if (result) { strcpy; strcat; } and automagically return NULL if malloc fails

[Fred Foo]

By "written in place", do you mean join(join("foo", "bar"), "baz")? That would leak to a helluva memory leak. Your second example isn't only ugly, but unsafe due to lack of error checking.

[Fred Foo]

Non-standard, but not too hard to implement in terms of snprintf. +1.

[Admin]

@pmg I always check for NULL when using malloc.

[Alexandre C.]

@larsman: no, I mean writing the malloc, strcpy and strcat entirely instead of the join function.

[Bernd Elkemann]

This code scans s1 3 times for and s2 twice to just find out their lengths. If you want it faster you only need to scan each of them once.

[Alexandre C.]

Yes, there is a performance penalty in writing "clear" code :)

[Alexandre C.]

@eznme: this is clear code which works. After profiling, this can indeed be optimized. I prefer this than hunting off-by-one errors.

[nass]

@AlexandreC. I am fuzzy about strings; doesn't this malloc need to be freed somewhere (outside the scope of the join function) ?

[nass]

@AlexandreC. perhaps it's a vital update in your answer then. I can 'see' many newbies using the function as is and forgetting to free the malloc completely.

[chux - Reinstate Monica]

Recommend if(result) { } around the two memcpy().