Identifying a failed null check

Question

I am working on a function that must read one line at a time from a file, give the file descriptor of said file as input.

Here is my get_next_line function, and helper functions on top of it

# include <fcntl.h>
# include <stdlib.h>
# include <unistd.h>
# include <stdio.h>


char    *addline(int fd, char *line)
{
        int             i;
        char    *tmp;
        int             bytes;

        i = 0;
        bytes = 1;
        if (read(fd, 0, 0) < 0 )
                return (NULL);
        while (line[i] != '\n')
        {
                tmp = (char *) malloc((BUFFER_SIZE + 1) * sizeof(char));
                if (!tmp)
                        return (NULL);
                bytes = read(fd, tmp, BUFFER_SIZE);
                if (bytes == -1)
                        return (free(tmp), free(line), NULL);
                tmp[bytes] = '\0';
                line = ft_strjoin(line, tmp);
                if (bytes == 0)
                        return (line);
                i = 0;
                while (line[i] && line[i] != '\n')
                        i++;
        }
        return (line);
}

char    *ft_nextline(char *str, int i)
{
        int             j;
        char    *line;

        j = i;
        line = (char *) malloc((i + 1) * sizeof(char));
        if (!line)
                return (NULL);
        line[i] = '\0';
        while (i--)
                line[i] = str[i];
        i = 0;
        if (str[j] == '\0')
        {
                str[i] = '\0';
                return (line);
        }
        while (str[j])
        {
                str[i] = str[j];
                i++;
                j++;
        }
        str[i] = '\0';
        return (line);
}

char    *get_next_line(int fd)
{
        static char     *line;
        int                     i;

        if (fd < 0 || fd > 1023 || BUFFER_SIZE <= 0)
                return (NULL);
        if (!line)
                line = ft_strdup("");
        line = addline(fd, line);
        i = 0;
        if (!line)
                return (free(line), NULL);
        while (line[i] && line[i] != '\n')
                i++;
        if (line[i] == '\n')
                return (ft_nextline(line, ++i));
        else if (line[i] == '\0' && i > 0)
                return (ft_nextline(line, i));
        return (free(line), NULL);
}

I will basically read into the file BUFFER_SIZE elements ( integer defined in the header file, it will have different values like 1, 42, 10000 or 9999), join this read string with a line string and check for \n. If found it shall return this result, from which I will extract the line, then memmove the useful bytes (everything after \n) to the bytes I've extracted, null terminating the result.

This process, with enough frees in different places (especially in my strjoin function) will allow me to print a line, remember what i read from the next and continue this process until EOF.

Below my two utils functions, string duplicate and string join :

char    *ft_strdup(char *s)
{
        char    *duplicate;
        size_t  i;
        size_t  j;

        i = 0;
        j = 0;
        while (s[i])
                i++;
        duplicate = (char *) malloc((i + 1) * sizeof(char));
        if (!duplicate)
                return (NULL);
        duplicate[i] = '\0';
        while (s[j])
        {
                duplicate[j] = s[j];
                j++;
        }
        return (duplicate);
}

char    *ft_strjoin(char *s1, char *s2)
{
        int             i;
        int             j;
        char    *s3;

        i = 0;
        j = 0;
        while (s1[i])
                i++;
        while (s2[j])
                j++;
        if (i == 0 && j == 0)
                return (free(s1), free(s2), NULL);
        else if (i == 0 && j != 0)
                return (free(s1), s2);
        else if (j == 0 && i != 0)
                return (free(s2), s1);
        s3 = (char *) malloc((i + j + 1) * sizeof(char));
        if (!s3)
                return (NULL);
        s3[i + j] = '\0';
        while (j--)
                s3[i + j] = s2[j];
        while (i--)
                s3[i] = s1[i];
        return (free(s1), free(s2), s3);
}

I do not pass a check of a testor program called francinette

The test consists of a "NULL check" that doesn't pass. I understand that it's an edge case where my function should return NULL, but segfaults instead.

The input is empty.txt, which is empty.

How can my function return NULL on my own tests and not NULL on the tests of the francinette? The function is Get_Next_Line.

Below is the output of francinette that causes my problem.

BUFFER_SIZE: 1

Invalid fd          : 1.OK 2.OK 3.OK 4_LEAKS.OK 5_NULL_CHECK.OK 

empty.txt           : 1.OK 2.OK 3_LEAKS.OK 4.KO Segfault

Segmentation fault (core dumped)

make: \[Makefile:23: mandatory\] Error 139 (ignored)

BUFFER_SIZE: 10

Invalid fd          : 1.OK 2.OK 3.OK 4_LEAKS.OK 5_NULL_CHECK.OK 

empty.txt           : 1.OK 2.OK 3_LEAKS.OK 4.KO Segfault

Segmentation fault (core dumped)

make: \[Makefile:24: mandatory\] Error 139 (ignored)

BUFFER_SIZE: 1000000

Invalid fd          : 1.OK 2.OK 3.OK 4_LEAKS.OK 5_NULL_CHECK.OK 

empty.txt           : 1.OK 2.OK 3_LEAKS.OK 4.KO Segfault

Segmentation fault (core dumped)

make: \[Makefile:25: mandatory\] Error 139 (ignored)

What is the solution?

Your code does not segfault for me. I speculate that the test harness may expect the pointer returned by get_next_line() to be one that the caller has responsibility to free (so it frees it), which could cause a problem on subsequent calls to get_nextLine() because it invalidates that function's static copy of a pointer to the same space. Even if that turns out not to be the issue, though, static local variables are tricky and usually best avoided. Especially modifiable ones and those that get exposed outside the function to which they belong. — John Bollinger
– John Bollinger, Commented Nov 12 at 16:10
@Simeon, the cast and duplicate = (char *) malloc((i + 1) * sizeof(char)); are not needed in duplicate = (char *) malloc((i + 1) * sizeof(char));.. Consider duplicate = malloc(i + 1); unless you like WET. — chux
– chux, Commented Nov 13 at 2:55
@SimeonSutarmin, the issue with freeing the returned pointer is that get_next_line() does not know whether it has been freed, and assumes that it has not been. If its assumption is wrong then that leaves it dereferencing an invalid pointer. If it is the caller's responsibility to free the returned pointer, then the corresponding responsibility of get_next_line() is to allocate fresh space on every call. — John Bollinger
– John Bollinger, Commented Nov 13 at 14:41
The casts to malloc are not only unnecessary, but a bad habit, they tell the compiler to do no checking (and hide an error if you don't provide the appropiate #include <stdlib.h> in the head of the compilation unit) because you are smart enough and you know what you are doing. The compiler will silently interpret (well not silently, you will get just a warning if you are using C98, for example) and let you on your own. — Luis Colorado
– Luis Colorado, Commented Nov 14 at 12:12
You and many others learnt that way. And many insist still in that this is a bad sin and discuss it as if it was an act of faith. IMHO explaining why things are done this way (without an explanation) is the reason of having as many credoes in C programming. A hint would be don’t follow credoes and question why things are done that way. — Luis Colorado
– Luis Colorado, Commented Nov 16 at 13:30

dbush · Accepted Answer · 2025-11-13 01:53:21Z

1

The test framework is likely stubbing out the malloc function so it can test how your code handles the case when malloc returns NULL.

In both ft_strdup and ft_strjoin you're checking for a NULL return from malloc and if so those functions will in turn return NULL. However, you're not always checking whether or not these two functions return NULL.

In addline you have this:

            line = ft_strjoin(line, tmp);
            if (bytes == 0)
                    return (line);
            i = 0;
            while (line[i] && line[i] != '\n')

Where you use the return value of ft_strjoin without checking if it's NULL.

Similarly in get_next_line you have this:

    if (!line)
            line = ft_strdup("");
    line = addline(fd, line);

Which goes here in addline:

    i = 0;
    bytes = 1;
    if (read(fd, 0, 0) < 0 )
            return (NULL);
    while (line[i] != '\n')

Where you're using the return value of ft_strdup without checking if it's NULL.

Here's the fixed code of addline with the proper NULL checks:

char    *addline(int fd, char *line)
{
        int             i;
        char    *tmp;
        int             bytes;

        i = 0;
        bytes = 1;
/*HERE*/if (!line) return NULL;
        if (read(fd, 0, 0) < 0 )
                return (NULL);
        while (line[i] != '\n')
        {
                tmp = (char *) malloc((BUFFER_SIZE + 1) * sizeof(char));
                if (!tmp)
                        return (NULL);
                bytes = read(fd, tmp, BUFFER_SIZE);
                if (bytes == -1)
                        return (free(tmp), free(line), NULL);
                tmp[bytes] = '\0';
                line = ft_strjoin(line, tmp);
/*HERE*/        if (!line || bytes == 0)
                        return (line);
                i = 0;
                while (line[i] && line[i] != '\n')
                        i++;
        }
        return (line);
}

Note that this code still has memory leaks. Fixing those is left as an exercise to the reader.

answered Nov 13 at 1:53

dbush

233k27 gold badges261 silver badges334 bronze badges

Sign up to request clarification or add additional context in comments.

5 Comments

Simeon Sutarmin Nov 13 at 12:56

the line check in the addline function turned my segfault into a KO of the NULL check, so it is an improvement and i understand why. The problem however, persists. Thank you for this improvement tho its totally a valid addition to the function.

Carl HR Nov 14 at 3:59

There are two things I've found: 1. Inside get_next_line, when you declare line, you're not initializing any value to it. This is a problem, as C doesn't default values to zero/null. This variable can hold garbage, leading to undefined behavior. You should initialize it as null in order to avoid errors; 2. Inside ft_strjoin, right when you check if (!s3) and return null, you should return (free(s1), free(s2), NULL), to ensure that all pointers are released from memory, like your last return on this scope.

Simeon Sutarmin Nov 14 at 11:10

Thank you for this improvements ! I will maybe update my post because i have implemented some changes and still my NULL check doesnt pass. But it is important to add these improvements, thank you.

Carl HR Nov 14 at 4:05

Also, 3. Inside ft_nextline, you should free str, unless you plan to keep the pointer alive when the function returns. Its not the case, as you only call this function on a return statement, for example here return (ft_nextline(line, i));. In this case, line is str, and it's not going to be used anymore, leading to a memory leak as you never released it from memory.

Simeon Sutarmin Nov 14 at 11:16

For the 3rd point i believe it is freed on the next call of Get_Next_Line. Basically it will give back line(ft_nextline), which is a copy off str(ft_nextline), which itself if line(get_next_line). So on the next call of get_next_line: 1. addline will return 0 and stop 2. the [i] index of get_next_line will then count a null terminator on line(get_next_line) at position i = 0 3. it will be falsle on if (line[i] == '\n') / else if (line[i] == '\0' && i > 0) and enter the third else -> return (free(line), NULL). This for me is supposed to be the free of this line. Right?

Collectives™ on Stack Overflow

Identifying a failed null check

1 Answer 1

5 Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

5 Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related