Search code examples
cgnu

free(): invalid next size (normal) while executing free


I am learning c programming. Below program is showing me the output, But when it execute free method . It is give me error:- free(): invalid next size (normal) . Please let me know what i am missing.

 #include<stdio.h>
    #include<stdlib.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*) calloc(alloc_size , sizeof(char));
        int lenofbuff = 0;
        while((lenofbuff=fread(r,sizeof(char),1024,fp))>0){

            totallengthread += lenofbuff;
                    if (totallengthread >= alloc_size) {
                        alloc_size += 1024;
                        buffer = realloc(buffer, alloc_size);
                    }
            concat(buffer, r);
        }
        printf("this is the output =>%s", buffer);
        pclose(fp);
        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';
    }

Solution

    1. In modern C, routines must be declared before they are used. Either move the definition of concat before main or insert a declaration of concat before main.

    2. Change int main() to int main(void).

    3. fread does not add a null terminator to the data read. Change char r[1024]; to char r[1025]; and, after fread, insert r[lenofbuff] = '\0'; as the first statement inside the while body.

    4. if (totallengthread >= alloc_size) does not account for the null terminator. Change it to if (totallengthread+1 >= alloc_size)`.

    5. In concat, change char *s = source; to const char *s = source;.

    6. Turn on compiler warnings and pay attention to them. They should have warned you about 1 and 5 above.

    7. After char *buffer = (char*) calloc(alloc_size, sizeof(char));, test buffer == NULL. If it is, print an error and exit. Also, a better form for this statement is char *buffer = calloc(alloc_size, sizeof *buffer);. Casting the result of calloc is not needed in C, and basing the size on the thing being allocated rather than a repetition of the type may be safer if the type is changed in the future.

    8. Change buffer = realloc(buffer, alloc_size); to char *temp = realloc(buffer, alloc_size * sizeof *buffer); if (temp == NULL) { print message and exit } else buffer = temp;.