Custom concat function in C with pointer

Question

I try to code my own concatenation function in C without library, but I have issue and I don't know where it comes from. To do my function I use pointers of char.

This is my Code :

#include <stdio.h>
#include <stdlib.h>

int longueur(char *str)
{
    int i =0;
    while(str[i] != '\0')
    {
        i++;
    }
    return i;
}

void concat(char* source, char* dest)
{
    int longStr1 = (longueur(source));
    int longStr2 = (longueur(dest));
    int i=0, j=0;
    char* temp = dest;
    free(dest);
    dest = (char*) realloc(dest, ((longStr1 + longStr2)* sizeof(char)));

    /*dest[0] = temp[0]; <------  If I do this it will generate issue, so the bellow code too*/
    while(temp[i] != '\0')
    {
        dest[i] = temp[i];
        i++;
    }
    while(source[j] != '\0')
    {
        dest[i] = source[j];
        i++;
        j++;
    }
   dest[i] = '\0';
}

int main()
{
    char *str1 = "World";
    char *str2 = "Hello";
    concat(str1, str2);
    printf("-------------\n%s", str2);
    return 0;
}

EDIT

I read all your answer, so I changed my concat function to :

void concat(char* source, char* dest)
{
    int longStr1 = (longueur(source));
    int longStr2 = (longueur(dest));
    int i=0, j=0;

    dest = (char*) malloc((longStr1 + longStr2)* sizeof(char) + sizeof(char));

    while(dest[i] != '\0')
    {
        dest[i] = dest[i];
        i++;
   }
   while(source[j] != '\0')
   {
       dest[i] = source[j];
       i++;
       j++;
   }
   dest[i] = '\0';
}

Now I don't have issue but my code only display "Hello"

Answer 1

In your code

free(dest);

and

dest = (char*) realloc(dest, ((longStr1 + longStr2)* sizeof(char)));

invokes undefined behavior as none of them use a pointer previously allocated by malloc() or family.

Mostly aligned with your approach, you need to make use of another pointer, allocate dynamic memory and return that pointer. Do not try to alter the pointers received as parameters as you've passed string literals.

That said, you need to have some basic concepts clear first.

1. You need not free() a memory unless it is allocated through malloc() family.
2. You need to have a char extra allocated to hold the terminating null.
3. Please see this discussion on why not to cast the return value of malloc() and family in C..
4. If your concatenation function allocates memory, then, the caller needs to take care of free()-ing the memory, otherwise it will result in memory leak.

Answer 2

In addition to all the good comments and solutions: realloc can give you a different pointer and you must return that pointer. So your function signature should be:

void concat(char* source, char** dest)
{
    int longStr1 = (longueur(source));
    int longStr2 = (longueur(dest));
    int i=0, j=0;
    char* temp = *dest, *temp2;
    if ((temp2 = realloc(dest, ((longStr1 + longStr2)+1))==NULL) return;
    *dest= temp2;

    while(temp[i] != '\0')
    {
        *dest[i] = temp[i];
        i++;
    }
    while(source[j] != '\0')
    {
        *dest[i] = source[j];
        i++;
        j++;
    }
   *dest[i] = '\0';
}

..and this assumes the function will only be called with a dest that was allocated with malloc. And sizeof(char) is always 1. (This resulting function is not optimal.)

--EDIT--

Below the correct, optimized version:

void concat(char* source, char** dest)
{
    int longSrc = longueur(source);
    int longDst = longueur(dest);
    char *pDst, *pSrc;

    if ((pDst = realloc(*dest, longSrc + longDst + 1))==NULL) return;

    if (pDst != *dest) *dest= pDst;

    pDst += longSrc;
    pSrc= source;

    while(pSrc)
        *pDst++ = *pSrc++;

    *pDst = '\0';
}

Answer 3

After you have freed dest here:

free(dest);

You cannot use this pointer in following call to realloc:

dest = (char*) realloc(dest, ((longStr1 + longStr2)* sizeof(char)));

/*dest[0] = temp[0]; <------  If I do this it will generate issue, so the bellow code too*/

man realloc

void *realloc(void *ptr, size_t size);

The realloc() function changes the size of the memory block pointed to by ptr to size bytes. (...)

But this pointer is invalid now and you cannot use it anymore. When you call free(dest), the memory dest points to is being freed, but the value of dest stays untouched, making the dest a dangling pointer. Accessing the memory that has already been freed produces undefined behavior.

NOTE: Even if free(dest) is technically valid when called on pointer to memory allocated by malloc (it is not an error in your function to call free(dest) then), it is incorrect to use this on pointer to literal string as you do in your example (because str2 points to string literal it is an error to pass this pointer to function calling free on it).

Answer 4

Given your original use, perhaps you would find a variant like this useful

#include <stdio.h>
#include <stdlib.h>
#include <stdint.h>

size_t longueur ( const char * str ) {               /* correct type for string lengths */
    size_t len = 0;

    while (*str++ != '\0') ++len;
    return len;
}

char * concat ( const char * first, const char * second ) {
    const char * s1 = first  ? first  : "";          /* allow NULL input(s) to be */
    const char * s2 = second ? second : "";          /* treated as empty strings */
    size_t ls1 = longueur(s1);
    size_t ls2 = longueur(s2);
    char * result = malloc( ls1 + ls2 + 1 );         /* +1 for NUL at the end */
    char * dst = result;   

    if (dst != NULL) {
        while ((*dst = *s1++) != '\0') ++dst;        /* copy s1\0 */
        while ((*dst = *s2++) != '\0') ++dst;        /* copy s2\0 starting on s1's \0 */
    }
    return result;
}

int main ( void ) {
    const char *str1 = "Hello";
    const char *str2 = " World";
    char * greeting = concat(str1, str2);

    printf("-------------\n%s\n-------------\n", greeting);
    free(greeting);
    return 0;
}

In this variant, the two inputs are concatenated and the result of the concatenation is returned. The two inputs are left untouched.