I'm still writing my own implementation of get_next_line, a function that takes a file descriptor and outputs the line of the file, then the next line and so on, since my last post, but now everything is working in my code !
The problem is, in the norms I have to follow, my code have to be less than 25 lines, and my function is actually 30. I successfully split into one sub function but I need to split it one more time but this time, I get invalid frees or invalid reads and completely wrong results.
(I have a get_next_line_utils.c that contains homemade libc functions and it has a strjoin that frees the stash in parameter because it creates a new one)
Here is my working code :
static int check_line(char *str)
{
int i;
i = 0;
if (!str)
return (-1);
while (str[i])
{
if (str[i] == '\n')
return (1);
i ;
}
return (-1);
}
/*
ft_strcut is used in one case in GNL.
When the stash contains a \n, we are cutting the string
from the \n to the end of the string, so our new stash does not
longer contain the \n ans the past line.
*/
static char *ft_strcut(char *str)
{
char *cutted_str;
int i;
int j;
i = 0;
j = 0;
while (str[i] != '\n')
i ;
cutted_str = malloc(sizeof(char) * (ft_strlen(str) - i 1));
if (!cutted_str)
return (NULL);
i ;
while (str[i])
cutted_str[j ] = str[i ];
cutted_str[j] = '\0';
free(str);
return (cutted_str);
}
/*
I used this function to make GNL less than 25 lines.
This block of code was between the variable declaration and the while loop.
This function is checking the validity of fd, BUFFER_SIZE and the stash.
And it's initializing our stash for the rest of GNL if the verification passed.
*/
static char *stash_checking(int fd, char *stash, char *buff, int readed)
{
if (fd < 0 || BUFFER_SIZE <= 0)
return (NULL);
buff = malloc(sizeof(char) * (BUFFER_SIZE 1));
if (!buff)
return (NULL);
readed = read(fd, buff, BUFFER_SIZE);
if (readed <= 0 && !stash)
{
free(buff);
return (NULL);
}
buff[readed] = '\0';
if (!stash)
stash = ft_substr(buff, 0, ft_strlen(buff)); //It's like strdup
else
stash = ft_strjoin(stash, buff);
free(buff);
if (stash[0] == '\0')
{
free(stash);
stash = NULL;
return (NULL);
}
return (stash);
}
char *get_next_line(int fd)
{
static char *stash; //Static variable to keep the stash between calls.
char *buff; //Buffer to read the file.
char *line; //Line to return.
int readed; //Readed bytes.
readed = 0;
buff = NULL;
stash = stash_checking(fd, stash, buff, readed);
while (stash)
{
buff = malloc(sizeof(char) * (BUFFER_SIZE 1));
readed = read(fd, buff, BUFFER_SIZE);
buff[readed] = '\0';
stash = ft_strjoin(stash, buff);
free(buff);
if (readed < BUFFER_SIZE && check_line(stash) == -1)
{
line = ft_substr(stash, 0, ft_strlen(stash)); //It's like strdup
free(stash);
stash = NULL;
return (line);
}
else if (check_line(stash) != -1)
{
line = ft_substr(stash, 0, ft_strchr(stash, '\n') - stash 1);
stash = ft_strcut(stash);
return (line);
}
}
return (NULL);
}
As you can see, I successfully split a chunk of code into a function named stash_checking to make my code shorter but it is not enough.
I tried to put the lines from if (readed < BUFFER_SIZE && check_line(stash) == -1) to the end of else if
in a sub function that I named handling_cases and I'm getting a double free() error.
static char *handling_cases(int readed, char *stash, char *line)
{
if (readed < BUFFER_SIZE && check_line(stash) == -1)
{
line = ft_substr(stash, 0, ft_strlen(stash));
free(stash);
stash = NULL;
return (line);
}
else if (check_line(stash) != -1)
{
line = ft_substr(stash, 0, ft_strchr(stash, '\n') - stash 1);
stash = ft_strcut(stash);
return (line);
}
return (NULL);
}
char *get_next_line(int fd)
{
static char *stash; //Static variable to keep the stash between calls.
char *buff; //Buffer to read the file.
char *line; //Line to return.
int readed; //Readed bytes.
readed = 0;
buff = NULL;
stash = stash_checking(fd, stash, buff, readed);
while (stash)
{
buff = malloc(sizeof(char) * (BUFFER_SIZE 1));
readed = read(fd, buff, BUFFER_SIZE);
buff[readed] = '\0';
stash = ft_strjoin(stash, buff);
free(buff);
line = handling_cases(readed, stash, line);
if (line)
return (line);
}
return (NULL);
}
I have tested a lot of similar things, putting all the while loop in a sub function that returns the line and many different things but I don't understand why, I'm getting some double frees, invalid write or not expected output.
I would like to understand why putting the exact chunk of code in a separate function is doing errors.
I debugged my code, and it seem's my stash is not correctly freed, or always have no values in it when I call GNL at the end of the file.
I'm sorry if this is a little confusing, my english is not perfect yet..
Any help is much appreciated !
CodePudding user response：
Your approach with passing stash is kind-of correct, but you missed one thing: You need to pass a pointer to the variable to a called function. This resembles "pass by reference" in C, which only has "pass by value".
It does not matter, whether such a variable is static or not. The called function does not need to know, and you cannot make it know.
I have simplified your example and used generic names:
#include <stdio.h>
#include <stdlib.h>
static void called(int **pointer_to_pointer) {
int value = 42;
if (*pointer_to_pointer != NULL) {
value = **pointer_to_pointer;
//free(*pointer_to_pointer);
}
*pointer_to_pointer = malloc(sizeof **pointer_to_pointer);
**pointer_to_pointer = value 1;
printf("%s(): %p -> %p -> %d\n", __FUNCTION__, (void*)pointer_to_pointer, (void*)*pointer_to_pointer, **pointer_to_pointer);
}
static void caller(void) {
static int *pointer = NULL;
int value = 23;
if (pointer != NULL) {
value = *pointer;
//free(pointer);
}
pointer = malloc(sizeof *pointer);
*pointer = value 1;
printf("%s(): @ %p: %p -> %d\n", __FUNCTION__, (void*)&pointer, (void*)pointer, *pointer);
called(&pointer);
printf("%s(): @ %p: %p -> %d\n", __FUNCTION__, (void*)&pointer, (void*)pointer, *pointer);
called(&pointer);
printf("%s(): @ %p: %p -> %d\n", __FUNCTION__, (void*)&pointer, (void*)pointer, *pointer);
}
int main(void) {
printf("%s()\n", __FUNCTION__);
caller();
printf("%s()\n", __FUNCTION__);
caller();
printf("%s()\n", __FUNCTION__);
}
Some notes:
- Because in my environment
malloc()returned exactly the same memory that was justfree()ed, I needed to comment those calls to show that new memory was allocated. - The size in the calls of
malloc()uses thesizeofoperator with the expression that "is" the wanted object. You can keep using the parenthesized version of this operator with a data type, likesizeof (int)for this demonstration program. - Here is no error check after calling
malloc(). Don't do this in production code. __FUNCTION__is a nice predefined preprocessor macro of GCC and Clang, which expands to the name of the current function as a C string.printf()expectsvoid*for its format specifier"%p", so every pointer value is correctly cast.