Modify char array passed to a function, C

Question

I would like to concat 2 strings inside a function. However, I would like the function to modify also the destination string (char array).

So far I got this code below, but it shows 'segmentation fault' and I do not know how can I fix this. Thanks.

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

char *my_strcat(char *dest, const char * src) {
    char *tab = (char*) malloc(sizeof(char) * (strlen(*dest) + strlen(src) + 1));
    if(NULL == tab)
        return NULL;
    strcpy(tab, dest);
    strcpy(tab, src);
    free(dest);
    dest = (char*)malloc(sizeof(char) * (strlen(tab)+1));
    strcpy(dest, tab);
    return tab; }

int main(int argc, char **argv) {
    char *dst = NULL, *src = NULL, *r = NULL;
    int i;
    src = malloc(sizeof(char) * 100);
    strcpy(src, "fifty");

    dst = malloc (sizeof (char *) * 100);
    strcpy(src, "four");

    r = my_strcat(dst, src);
    printf("%s\n", r);
    printf("%s\n", dst);

    free(r);
    free(dst);
    free(src);

    return 0; }

Answer 1

dst is allocated in main. To modify that allocation in a function, you can return the new pointer or pass a pointer to pointer. Since you return another pointer, you must pass a pointer to pointer char **dest and make a few changes in the function to accommodate that change.

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

char *my_strcat ( char **dest, const char * src) {
    char *tab = malloc ( sizeof ( char) * (strlen ( *dest) + strlen ( src) + 1));//dereference dest
    if ( NULL == tab)
        return NULL;
    strcpy ( tab, *dest);//dereference dst
    strcat ( tab, src);
    free ( *dest);//dereference dest
    *dest = malloc ( sizeof ( char) * ( strlen ( tab) + 1));//dereference dest
    strcpy ( *dest, tab);//dereference dest
    return tab;
}

int main(int argc, char **argv) {
    char *dst = NULL, *src = NULL, *r = NULL;
    src = malloc ( sizeof ( char) * 100);
    strcpy ( src, "fifty");

    dst = malloc ( sizeof ( char) * 100);
    strcpy ( dst, "four");

    r = my_strcat ( &dst, src);//use address of dst
    printf ( "%s\n", r);
    printf ( "%s\n", dst);

    free ( r);
    free ( dst);
    free ( src);

    return 0;
}

Answer 2

There are several problems in your code. You can use tools like valgrind, clang-tidy to help you.

Here is your code with line numbers

 1  /* -*- compile-command: "gcc prog.c; ./a.out"; -*- */
 2  #include <stdio.h>
 3  #include <stdlib.h>
 4  #include <string.h>
 5  
 6  char *my_strcat(char *dest, const char *src)
 7  {
 8    char *tab = (char *)malloc(sizeof(char) * (strlen(*dest) + strlen(src) + 1));
 9    if (NULL == tab)
10      return NULL;
11    strcpy(tab, dest);
12    strcpy(tab, src);
13    free(dest);
14    dest = (char *)malloc(sizeof(char) * (strlen(tab) + 1));
15    strcpy(dest, tab);
16    return tab;
17  }
18  
19  int main(int argc, char **argv)
20  {
21    char *dst = NULL, *src = NULL, *r = NULL;
22    int i;
23    src = malloc(sizeof(char) * 100);
24    strcpy(src, "fifty");
25  
26    dst = malloc(sizeof(char *) * 100);
27    strcpy(src, "four");
28  
29    r = my_strcat(dst, src);
30    printf("%s\n", r);
31    printf("%s\n", dst);
32  
33    free(r);
34    free(dst);
35    free(src);
36  
37    return 0;
38  }

Let's use cland-tidy to perform a static analysis:

clang-tidy-7 prog.c --

prints a bunch of messages:

8 warnings generated.
/home/picaud/Temp/prog.c:8:46: warning: 1st function call argument is an uninitialized value [clang-analyzer-core.CallAndMessage]
  char *tab = (char *)malloc(sizeof(char) * (strlen(*dest) + strlen(src) + 1));
                                             ^
/home/picaud/Temp/prog.c:26:9: note: Storing uninitialized value
  dst = malloc(sizeof(char *) * 100);
        ^
/home/picaud/Temp/prog.c:29:7: note: Calling 'my_strcat'
  r = my_strcat(dst, src);
      ^
/home/picaud/Temp/prog.c:8:46: note: 1st function call argument is an uninitialized value
  char *tab = (char *)malloc(sizeof(char) * (strlen(*dest) + strlen(src) + 1));
                                             ^
/home/picaud/Temp/prog.c:8:53: warning: incompatible integer to pointer conversion passing 'char' to parameter of type 'const char *';

etc...

The first lines explain that dest is not initialized, you can track back it to:

26    dst = malloc(sizeof(char *) * 100);
27    strcpy(src, "four");

which is certainly a bug and have to be remplaced by

26    dst = malloc(sizeof(char *) * 100);
27    strcpy(dst, "four");

Now you can rerun clang-tidy. The first message is:

/home/picaud/Temp/prog.c:8:53: warning: incompatible integer to pointer conversion passing 'char' to parameter of type 'const char *';
remove * [clang-diagnostic-int-conversion]
  char *tab = (char *)malloc(sizeof(char) * (strlen(*dest) + strlen(src) + 1));             
                                                    ^~~~~

Which immediatly points to another bug, replace:

8     char *tab = (char *)malloc(sizeof(char) * (strlen(*dest) + strlen(src) + 1));

by

8     char *tab = (char *)malloc(sizeof(char) * (strlen(dest) + strlen(src) + 1));

you also have

/home/picaud/Temp/prog.c:26:9: warning: Result of 'malloc' is converted to a pointer of type 'char', which is incompatible with sizeof operand type 'char *' [clang-analyzer-unix.MallocSizeof]
  dst = malloc(sizeof(char *) * 100);
        ^

hence you must replace:

  dst = malloc(sizeof(char *) * 100);

by

  dst = malloc(sizeof(char) * 100);

reruning clang-tidy still shows some problems:

 /home/picaud/Temp/prog.c:27:3: note: Call to function 'strcpy' is insecure as it does not provide bounding of the memory buffer. Replace unbounded copy functions with analogous functions that support length arguments such as 'strlcpy'. CWE-119
/home/picaud/Temp/prog.c:31:3: warning: Use of memory after it is freed [clang-analyzer-unix.Malloc]
  printf("%s\n", dst);
  ^
/home/picaud/Temp/prog.c:26:9: note: Memory is allocated
  dst = malloc(sizeof(char) * 100);
        ^
/home/picaud/Temp/prog.c:29:7: note: Calling 'my_strcat'
  r = my_strcat(dst, src);
      ^
/home/picaud/Temp/prog.c:9:3: note: Taking false branch
  if (NULL == tab)
  ^
/home/picaud/Temp/prog.c:13:3: note: Memory is released
  free(dest);
  ^
/home/picaud/Temp/prog.c:29:7: note: Returning; memory was released via 1st parameter
  r = my_strcat(dst, src);
      ^
/home/picaud/Temp/prog.c:31:3: note: Use of memory after it is freed
  printf("%s\n", dst);

I let you check all this. On my side following these warnings I get this new version for your code:

/* -*- compile-command: "gcc prog.c; ./a.out"; -*- */
#include <stdio.h>
#include <stdlib.h>
#include <string.h>

char *my_strcat(char *dest, const char *src)
{
  char *tab = (char *)malloc(sizeof(char) * (strlen(dest) + strlen(src) + 1));
  if (NULL == tab)
    return NULL;
  strcpy(tab, dest);
  strcpy(tab+strlen(dest), src);
  return tab;
}

int main(int argc, char **argv)
{
  char *dst = NULL, *src = NULL, *r = NULL;
  int i;
  src = malloc(sizeof(char) * 100);
  strcpy(src, "fifty");

  dst = malloc(sizeof(char) * 100);
  strcpy(dst, "four");

  r = my_strcat(dst, src);
  printf("%s\n", r);
  printf("%s\n", dst);

  free(r);
  free(dst);
  free(src);

  return 0;
}

when run, this code prints:

gcc prog.c; ./a.out
fourfifty
four

You can use valgrind to check that there is no memory leaks:

valgrind ./a.out 
==4023== Memcheck, a memory error detector
==4023== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==4023== Using Valgrind-3.14.0 and LibVEX; rerun with -h for copyright info
==4023== Command: ./a.out
==4023== 
fourfifty
four
==4023== 
==4023== HEAP SUMMARY:
==4023==     in use at exit: 0 bytes in 0 blocks
==4023==   total heap usage: 4 allocs, 4 frees, 1,234 bytes allocated
==4023== 
==4023== All heap blocks were freed -- no leaks are possible
==4023== 
==4023== For counts of detected and suppressed errors, rerun with: -v
==4023== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

Good learning!