Segmentation Fault caused by realloc?

Question

Hei, I was trying to solve this school exercise..

Write a program that keeps reading in character strings and concatenates them (adds them to a single character string). the concatenation should take place in a function that returns 1 if successful or 0 if it fails. for memory allocation use only realloc!

I don't receive any error while debugging the program, but when I try to run the program, after I insert the string the only thing that appears is "Segmentation Fault", what could it be? This is the code:

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

int cat(char **, char *);

int main(void)
{
  char string[51];
  char *output=NULL;
  char choice;
  do
  {
    printf("Please enter a string [<50 chars]: ");
    fgets(string,50,stdin);
    if(string[strlen(string)-1]=='\n') /* if newline was read as well */
      string[strlen(string)-1]=0;      /* discard it */
    if(cat(&output,string))
      printf("\n\nThe string now contains:\n%s\n",output);
    else
    {
      printf("error: memory (re-)allocation failed!\n\n");
      return 1; /* exit with error */ 
    }
    printf("Continue? (y/n) - ");
    fgets(string,3,stdin); /* read input from keyboard - leave a safety buffer to account for read newline */
    choice=string[0]; /* use the first character from the previous read as the choice */
  } while(choice=='y' || choice=='Y');

  free(output);
  return 0;
}

int cat(char **dest, char *src)
{

  int i;
  int length1=strlen(src);
  int length2=strlen(*dest);
  int length3=length1+length2;
  *dest=(char*)realloc(NULL,sizeof(*src));
  printf("%p", *dest);
  if(*dest==NULL) return 0; /* if allocation failed */
  for(i=0;i<=length3;i++)
  {
      if(i<=length1)
        (*dest)[i]=(*dest)[i];
      else
        (*dest)[i]=(src)[i];
  }
  free(src);
  return 1;
}

Answer 1

There are at least 5 issues with your code:

1) You should free only what you allocated yourself on the heap. Don't free(src) because what you pass in src points to stack memory (char string[51]; is freed automatically).

2) you probably meant to reallocate dest, and 3) you meant to allocate memory the size of length3 (+1 null-terminator).

    *dest=(char*)realloc(*dest, length3 + 1);

4) strlen(*dest) will crash when *dest is NULL initially.

    int length2=(*dest)?strlen(*dest):0;

5) I don't think your for-loop is correct. It won't concatenate the strings, your offset calculation is off.