Sad Adl
Sad Adl

Reputation: 45

getline into char** yields only the last line

I'm trying to store the line from getline(..., line, ...) into char** array. But when I iterate through the array to print the lines, it only prints the last line. I think there's something in the man that I missed.

the input file

standard output

int fill_map(t_args *args)
{
    char *line = NULL;
    args->maze = NULL;
    int i = 0;
    size_t len = 0;
    ssize_t nread;

    args->maze = malloc(sizeof(char *));
    for (size_t j = 0; (nread = getline(&line, &len, args->stream)) != -1; i++)
    {
        if (i == 1)
            args->width = (int)nread;
        args->maze = realloc(args->maze, sizeof(char *) * (i + 1));
        args->maze[i] = line;
    }
    args->height = i;

    for (size_t i = 0; i < (size_t)args->height; i++)
    {
        printf("%ld %s", i, args->maze[i]);
    }

    return nread;
}

Upvotes: 1

Views: 41

Answers (2)

Konrad Rudolph
Konrad Rudolph

Reputation: 545508

Your current code is reusing the buffer line for reading, and getline only reallocates it if its size is too small to fit the next line to read (and, in that cases, frees the previous buffer!). As a consequence, successive lines will be stored in the same buffer, at the same address, which is assigned to maze[i].

To avoid this, you can reset line and len before each call to getline:

for (size_t j = 0; (nread = getline(&line, &len, args->stream)) != -1; i++)
{
    if (i == 1)
        args->width = (int)nread;
    args->maze = realloc(args->maze, sizeof(char *) * (i + 1));
    args->maze[i] = line;
    line = NULL;
    len = 0;
}

free(line); // Important!

Note that we need to free(line) after the last getline call, even though that call failed.

In addition I would protest this abuse of the for loop, though this is certainly a matter of style: since you’re not really iterating over the same variable that you’re declaring and updating, I’d separate the file reading and looping over i (and what’s the point of j anyway?), and use a while loop; this also makes the usage of i simpler (no i + 1 necessary):

while ((nread = getline(&line, &len, args->stream)) != -1) {
    ++i;
    args->width = (int) nread;
    args->maze = realloc(args->maze, sizeof(char *) * i);
    args->maze[i] = line;
    line = NULL;
    len = 0;
}

Upvotes: 1

0___________
0___________

Reputation: 67476

You need to allocate memory for every line as well. Then you need to copy the line.

    args->maze = realloc(args->maze, sizeof(char *) * (i + 1));
    args -> maze[i] = malloc(strlen(line) + 1);
    strcpy(args->maze[i], line);

Upvotes: 0

Related Questions