Need help cleaning memory leaks and resolving segmentation fault in C code using malloc and free

Question

I've been working on this code in C and it has lots of leaks that I don´t know how to clean.

What I want when I call get_next_line on main is for it to return the next line of a .txt file.

Can somebody help me pls?


size_t  ft_strlen(char *s)
{
    size_t  i;

    if (!s)
        return (0);
    i = 0;
    while (s[i] != '\0')
        i++;
    return (i);
}

char    *ft_strchr(char *s, int c)
{
    size_t  i;

    i = 0;
    if (!s)
        return (0);
    while ((s[i] != '\0') && (s[i] != (unsigned char)c))
        i++;
    if ((s[i] == (unsigned char)c) || (c == '\0'))
        return ((char *)&s[i]);
    return (0);
}

char    *ft_substr(char *s, unsigned int start, size_t len)
{
    size_t  i;
    size_t  j;
    char    *sub;

    i = 0;
    j = 0;
    if (start >= ft_strlen(s) || !s || !len)
    {
        sub = malloc(1 * sizeof(char));
        sub[0] = '\0';
        return (sub);
    }
    while (s[start + j] != '\0' && j < len)
        j++;
    if (s[start + j] != '\0')
        j++;
    sub = malloc((j + 1) * sizeof(char));
    if (!sub)
    {
        free(s);
        return (0); 
    }
    while (s[start] != '\0' && j > i)
        sub[i++] = s[start++];
    sub[i] = '\0';
    //free(s);
    return (sub);
}

char    *ft_strjoin(char *s1, char *s2)
{
    char    *cat;
    size_t  i;
    size_t  j;

    i = 0;
    j = 0;
    if (!s1)
    {
        s1 = malloc(sizeof(char) * 1);
        s1[0] = '\0';
    }
    cat = (char *)malloc((ft_strlen(s1) + ft_strlen(s2) + 1) * sizeof(char));
    if (!cat)
        return (NULL);
    while (s1[i] != '\0')
    {
        cat[i] = s1[i];
        i++;
    }
    while (s2[j] != '\0')
        cat[i++] = s2[j++];
    free(s1);
    cat[i] = '\0';
    return (cat);
}


char    *ft_content(int fd, char *content)
{
    char    *buf;
    int     buf_nb;

    buf_nb = 1;
    buf = (char *)malloc((BUFFER_SIZE + 1) * sizeof(char));
    if (!buf)
        return (0);
    while ((!ft_strchr(content, '\n')) && buf_nb != 0)
    {
        buf_nb = read(fd, buf, BUFFER_SIZE);
        if (buf_nb == -1)
        {
            free(buf);
            free(content);
            return (NULL);
        }
        buf[buf_nb] = '\0';
        content = ft_strjoin(content, buf);
    }
    free(buf);
    return (content);
}

char    *get_next_line(int fd)
{
    static char *content;
    char        *sub;
    int         i;
    int         len;

    i = 0;
    if (fd < 0 || BUFFER_SIZE <= 0)
        return (NULL);
    content = ft_content(fd, content);
    if (content == NULL)
        return (NULL);
    if (content[0] == '\0')
        return (NULL);
    while (content[i] != '\n' && content[i] != '\0')
        i++;
    sub = ft_substr(content, 0, i);
    len = ft_strlen(content);
    i++;
    content = ft_substr(content, i, len);
    return (sub);
}
int    main(void)
{
    int    a = open("exemplo.txt", O_RDONLY);
    printf("%s", get_next_line(a));
    printf("%s", get_next_line(a));
    printf("%s", get_next_line(a));
    printf("%s", get_next_line(a));
    return (0);
}

I've done several changes but I always get memory leaks and segmentation fault.

What I expected was just to read the file.

Run your code through valgrind. If you're mismanaging memory it will tell you where. — dbush, Jun 01 '23 at 15:48
Welcome to Stack Overflow! [don't cast malloc](https://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc) — Barmar, Jun 01 '23 at 15:49
The first time you call `ft_content`, `content` will be `NULL`. So `ft_strchr(content, '\n')` will try to search for newline in a null pointer. This causes the segmentation fault. — Barmar, Jun 01 '23 at 15:51
You have many different small function. Unit-test each one of these to pinpoint where the issue is. — Eugene Sh., Jun 01 '23 at 15:52
In `ft_strchr()`, you have: `size_t i; i = 0;` —— but it would be better to use `size_t i = 0;` instead. In general, you should initialize variables when you define them and don't define them until you're about to use them, either. Sometimes, the initialization will be via a function call where a pointer is passed to the function; then, the initialization is unnecessary. (For example: `int i; if (scanf("%d", &i) != 1) { …error handling… }`). — Jonathan Leffler, Jun 01 '23 at 15:56
Also in `ft_strchr()`, you have `return ((char *)&s[i]);` —— but the type of `&s[i]` is `char *`, so the cast is pointless. The cast might be relevant (necessary) if `ft_strchr()` took a `const char *` argument to be searched — then you're getting into interesting philosophical discussions — the C standard library function `strchr()` has that problem; it returns a non-const pointer to the string treated as a `const char *`; its signature is `char *strchr(const char *s, int c)`. — Jonathan Leffler, Jun 01 '23 at 16:03
In addition to valgrind, this would be a good time to learn to step through code with a debugger. — Barmar, Jun 01 '23 at 16:15
Questions seeking debugging help should generally provide a [mre] of the problem, which includes all `#include` directives. A missing `#include` directive can not only cause compile-time errors, but also run-time errors, for example if the compiler interprets a missing declaration as an implicit declaration and makes a false assumption about the type of a parameter. Also, posting all `#include` directives makes it easier for other people to run your program in order to test it. — Andreas Wenzel, Jun 01 '23 at 18:02
Questions seeking debugging help should generally provide a [mre] of the problem, which includes the exact input required to reproduce the problem. Without the proper input, it may be impossible for other people to reproduce the problem. — Andreas Wenzel, Jun 01 '23 at 18:05
@JonathanLeffler: Personally, I tend to prefer declaring variables in C89-style, at the top of a code block. That way, I have a nice overview of all variables that my code block uses and of their scopes, and if I want to look up the type of a variable, I only have to look in one place instead of having to read the entire function. However, this is a matter of taste. I agree with you that it would be better to initialize the variables inside the declaration. — Andreas Wenzel, Jun 01 '23 at 18:15
@AndreasWenzel: I'm not too fussed about "variable definitions at the top of a block" vs "variable definitions as needed within a block", but I do like to have the variables' scope minimized. And in general, I like variables to be initialized when defined. This is one of the big fixes in C++ compared with C — it has initializers which generally ensure that user-defined types are properly initialized when defined (for some loose definition of 'properly' which depends on the class's designer). — Jonathan Leffler, Jun 01 '23 at 20:24

score 1 · Answer 1 · edited Jun 01 '23 at 18:29

1

If you are using the compilers gcc or clang, you can try using the command-line option -fsanitize=leak when compiling the program, in order to detect any memory leaks.

See this page from the gcc manual for further information.

edited Jun 01 '23 at 18:29

Andreas Wenzel

22,760
4
24
39

answered Jun 01 '23 at 17:08

Tymenko Maxsim

21
3

Need help cleaning memory leaks and resolving segmentation fault in C code using malloc and free

1 Answers1