Segmentation fault / Incorrect checksum triggered ocasionally when reading lines from a file

Question

I'm trying to implement a function that gets every line from a file and prints it. For some reason, when running it I get a segmentation fault sometimes, sometimes it just runs fine, sometimes it exits with a incorrect checksum for freed object. I don't know which pointer is not being freed/modified after freeing it, can I get some clue?

The variable BUFFER_SIZE is defined from the keyboard, where I compile with the flags gcc -Wall -Werror -Wextra -D BUFFER_SIZE=23 get_next_line.c main.c

    #include <string.h>
    #include <stdlib.h>
    #include <sys/types.h>

    char    *ft_strjoin(char const *s1, char const *s2)
    {
        char    *s;
        int     p;
        int     p2;
    
        if (s1 == NULL || s2 == NULL)
            return (NULL);
        s = malloc(strlen(s1) + strlen(s2) + 1);
        if (s == NULL)
            return (NULL);
        p = 0;
        while (s1[p])
        {
            s[p] = s1[p];
            p++;
        }
        p2 = 0;
        while (s2[p2])
        {
            s[p] = s2[p2];
            p++;
            p2++;
        }
        s[p] = '\0';
        return (s);
    }

    char    *ft_newstatic(char *aux, char *l, char **line)
    {
        char    *temp;
        int     leng;
    
        leng = strlen(l);
        temp = malloc(leng + 1);
        strlcat(temp, l, strlen(temp) + (leng - strlen(aux)) + 1);
        *line = strdup(temp);
        free(temp);
        l = NULL;
        l = strdup(&aux[1]);
        return (l);
    }
    
    int     get_next_line(int fd, char **line)
    {
        static char *stc_line;
        char        *buffer;
        ssize_t     nbytes;
        char        *aux;
    
        stc_line = (!stc_line) ? strdup("") : stc_line;
        buffer = malloc(BUFFER_SIZE + 1);
        if (!buffer || fd <= 0 || BUFFER_SIZE <= 0 || !line)
            return (-1);
        while ((nbytes = read(fd, buffer, BUFFER_SIZE)) > 0)
        {
            buffer[nbytes] = '\0';
            stc_line = ft_strjoin(stc_line, buffer);
            if ((aux = strchr(stc_line, '\n')) != NULL)
            {
                //free(buffer);
                return (((stc_line = ft_newstatic(aux, stc_line, line)) != NULL) ? 1 : 1);
            }
        }
        if (nbytes < 0)
            return (-1);
        if ((aux = strchr(stc_line, '\n')) != NULL)
        {
            //free(buffer);
            return (((stc_line = ft_newstatic(aux, stc_line, line)) != NULL) ? 1 : 1);
        }
        *line = strdup(stc_line);
        stc_line = NULL;
        free(buffer);
        return (0);
    }

If I remove both free(buffer) before returning ft_newstatic I get better results (less running time and not as many seg faults), but if I remove

stc_line = NULL;
free(stc_line);

I get weird output again:

2- archivo: texto.txt

[Return: 1] Line #124458: En un lugar de la mancha

[Return: 1] Line #124459: de cuyo nombre no quiero acordarme

[Return: 1] Line #124460: habia un hidalgo de los de lanza en astillero

[Return: 1] Line #124461: adarga antigua

[Return: 1] Line #124462: rocín flaco

[Return: 0] Line #124463: y galgo corredor.

End of file

3- archivo: texto2.txt

[Return: 1] Line #124464: y galgo corredor.linea 1

[Return: 1] Line #124465: linea 2

[Return: 1] Line #124466: linea 3

[Return: 0] Line #124467: linea 4

End of file

This is the main() that I use

    #include <sys/types.h>
    
    int get_next_line(int fd, char **line);
    
    int main(int argc, char **argv)
    {
        int fd;
        int ret;
        int line;
        char *buff;
        int     i;
    
        line = 0;
        if (argc > 1)
        {
            i = 0;
            while (++i < argc)
            {
                fd = open(argv[i], O_RDONLY);
                printf("%d- archivo: %s\n",i, argv[i]);
                while ((ret = get_next_line(fd, &buff)) > 0)
                {
                    printf("[Return: %d] Line #%d: %s\n", ret, ++line, buff);
                    free(buff);
                }
                printf("[Return: %d] Line #%d: %s\n", ret, ++line, buff);
                if (ret == -1)
                    printf("-----------\nError\n");
                else if (ret == 0)
                    printf("-----------\nEnd of file\n");
                free(buff);
            }
        }
        if (argc == 1)
        {
            while ((ret = get_next_line(0, &buff)) > 0)
                printf("[Return: %d] Line #%d: %s\n", ret, ++line, buff);
            if (ret == -1)
                printf("-----------\nError\n");
            else if (ret == 0)
                printf("-----------\nEnd of stdin\n");
            free(buff);
            close(fd);
        }
        return (0);
    }

Answer 1

temp = malloc(leng + 1);
strlcat(temp, l, strlen(temp) + (leng - strlen(aux)) + 1);

This is clearly wrong. The memory allocated by malloc contains garbage, so calling strlen on it makes no sense, and trying to use strlcat to append to it will break horribly. For instance, it is likely that the space you allocated doesn't contain a trailing nul byte, in which case strlen(temp) will return a number potentially much larger than leng+1, and strlcat may therefore write beyond the memory allocated. Or, strlen(temp) may simply segfault.

(I found this using valgrind, and then noticed that user3629249 also noticed the bug in a comment. I strongly recommend that you learn to use valgrind or a similar tool before proceeding any further with C.)

I cannot tell from your code what this function is supposed to do; it has an uninformative name and no comments. But I think you should first specify carefully its intended behavior, and then rewrite it.

As another note, you have a serious memory leak in main in the argc == 1 case:

        while ((ret = get_next_line(0, &buff)) > 0)
            printf("[Return: %d] Line #%d: %s\n", ret, ++line, buff);
        /* ... */
        free(buff);

Additional space is allocated for every line, and never freed. Only the last one is freed (outside the loop). When I ran your program, it used several gigabytes of memory for a modestly sized input file.

There are several other memory leaks in get_next_line where you use strdup to allocate strings that you never free.