Realloc existing data is getting lost

Question

I am learning c programming language. The program below takes the output of a command to variable and prints it. But when reallocate the memory to extend the memory I am losing the old data.

Can you please telling what I am missing here?

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

int main(){

FILE *fp;

char r[1024];
fp=popen("/bin/ls /etc/","r");

if(fp==NULL){
    perror("Failed to run the command");
    exit(-1);
}

int totallengthread=0,alloc_size=1024;
char *buffer = (char*) malloc(alloc_size*sizeof(char));
memset(buffer,0,sizeof(buffer));
int lenofbuff=0;
while(fgets(r,1023,fp)!=NULL){
 concat(buffer,r);
 lenofbuff=strlen(buffer);
 totallengthread+=lenofbuff;
 if(totallengthread>=alloc_size){
     alloc_size+=1024;
     realloc(buffer,alloc_size);
 }

}
pclose(fp);
printf("this is the out put =>%s",buffer);
free(buffer);
 return 0;
}
void concat(char *dest, const char *source){
    char *d=dest;
    char *s=source;
    while (*d != '\0') {
              d++;
           }
     while(*s!='\0')
        {
            *d++=*s++;
        }
    *d='\0';
}

Answer 1

I think this is a bit closer to what you want. Please see the comments in the code.

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

void concat(char *dest, const char *source){
    char *d=dest;
    char *s=source;
    while (*d != '\0'){
        d++;
    }
    while(*s!='\0'){
        *d++=*s++;
    }
    *d='\0';
}

int main(){
    FILE *fp;
    char r[1024];
    fp=popen("/bin/ls /etc/","r");
    if(fp==NULL){
        perror("Failed to run the command");
        exit(-1);
    }
    int totallengthread=0,alloc_size=1024;
    char *buffer = malloc(alloc_size*sizeof*buffer); //This is nicer because is type independent
    memset(buffer,'\0',sizeof*buffer); //I believe this is what you wanted.
    int lenofbuff=0;
    while(fgets(r,sizeof r,fp)){ // as @chux said this avoids hardcoding 1024-1. You also don't need to compare to NULL. 
        concat(buffer,r);
        lenofbuff=strlen(buffer);
        totallengthread+=lenofbuff;
        if(totallengthread>=alloc_size){
            alloc_size+=1024;
            buffer = realloc(buffer,alloc_size); //you need to use the return of realloc
        }
    }
    pclose(fp);
    printf("this is the out put =>%s",buffer);
    free(buffer);
    return 0;
}

As @chux mentioned, as in any memory allocation you should check whether the returned pointers are NULL, that would mean the allocation failed.

Also, please note that

memset(buffer,'\0',sizeof*buffer);

is equivalent to

memset(buffer,0,sizeof*buffer);

and as @thebusybee mentioned this is only setting the first element of the array to the NULL character, if you were intended to fill the whole array you should do memset(buffer,0,alloc_sizesizeofbuffer);

or just replacing

char *buffer = malloc(alloc_size*sizeof*buffer);
memset(buffer,0,alloc_size*sizeof*buffer);

for

char *buffer = calloc(alloc_size,sizeof*buffer);