cwhile-loopgetline

Is using getline() in a while loop bad practice?


If I have a file which contains a '\n' on virtually every line, and I use getline() in a while loop to read the content, is this bad practice? My understanding is that getline() will be called numerous times for each '\n' char reached, and so realloc() will be called each time also. Which seems inefficient?

#include <stdio.h>
#include <stdlib.h>
#include <string.h>

int main(int argc, char **argv) {
    FILE *fp = fopen("file.txt", "r");
    if (fp == NULL) {
        perror("Unable to open file: ");
        exit(1);
    }
    char *buffer = NULL;
    size_t len = 0;
    ssize_t characters;

    while ((characters = getline(&buffer, &len, fp)) != -1) {
        fputs(buffer, stdout);
    }
    fclose(fp);
    free(buffer);
    buffer = NULL;
    return 0;
}

Solution

  • There is no problem calling getline in a loop the way you do. As a matter of fact, this exactly the intended use case and used in the example in the man page.

    realloc is only called if the array allocated so far is too short for the current line, thus it will be called very little and you can even lower the number of calls by allocating an initial array and set its length in the len variable.

    Since getline returns the number of characters read, you could write the line using fwrite instead of fputs. Note however that the behavior is subtly different: fwrite will output lines read from the file containing embedded null bytes, whereas fputs would stop on the first null byte. This is usually not an issue as text files usually do not contain null bytes.

    Here is a modified version:

    #include <errno.h>
    #include <stdio.h>
    #include <stdlib.h>
    #include <string.h>
    
    int main(int argc, char **argv) {
        FILE *fp = fopen("file.txt", "r");
        if (fp == NULL) {
            fprintf(stderr, "Unable to open file %s: %s\n",
                    "file.txt", strerror(errno));
            return 1;
        }
        char *buffer = NULL;
        size_t len = 0;
        ssize_t characters;
    
    #if 1
        // optional code to show how to start with a preallocated buffer
        if ((buffer = malloc(256)) != NULL)
            len = 256; 
    #endif
    
        while ((characters = getline(&buffer, &len, fp)) != -1) {
            fwrite(buffer, 1, characters, stdout);
        }
        fclose(fp);
        free(buffer);
        return 0;
    }