Search code examples
cstringrealloc

C - character concatenate to string


I have the following simple program that reads character by character a text file. Each time a character is read from the file, must go at the end of the str which is a string. For this reason I made a small function called conc which takes the character, reallocates the str and then takes the character at the end of the string (str[count] = ch).
After the EOF character, I put the '\0' character to the str as an end for the string variable. My question is why the last printf displays garbage?? Any ideas?? Thanks in advance.

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

void conc(char* str,char ch,int* count);

int main(int argc,char** argv)
{
   char* str = (char*)malloc(sizeof(char));
   char ch;
   int count = 0,i;
   FILE* fp = fopen("path","r");

   if(fp == NULL){
       printf("No File Found");
       return 1;
   }

   ch=fgetc(fp);
   while(ch!=EOF){
       conc(str,ch,&count);
       ch=fgetc(fp);
   }

   str[count] = '\0';
   printf("%s",str);

   free(str);

   return(0);
}

void conc(char* str,char ch,int* count){
    str[*count] = ch;
    (*count)++;
    //printf("\n%c",str[(*count)-1]);
    str = (char*)realloc(str,(*count)+1);
}

Solution

  • The problem is with how you realloc() the pointer. The changes you do to str will not modify the original pointer in main(). You are only assigning to the copy of the pointer str in conc(). You need to pass a pointer to pointer in order to modify it.

    void conc(char** str,char ch,int* count){
        (*str)[*count] = ch;
        (*count)++;
        *str = realloc(*str,(*count)+1);
    }
    

    and pass a pointer to it from main():

    conc(&str,ch,&count);
    

    Change the prototype to match:

    void conc(char** str,char ch,int* count);
    

    Others notes:

    1) when realloc() fails it returns NULL and you will lose the original pointer. So you need to use a temporary and assign to the original. See: Having dynamically allocated an array can I change its size?

    2) Casting malloc()/realloc() etc is also dangerous.

    3) Always check the return value of malloc() etc to see if the memory allocation failed.

    3) Allocating one char at a time is not very efficient. Typical way is to allocate a buffer of size N and double the size when you realloc().