HiveBrain v1.2.0
Get Started
← Back to all entries
patterncMinor

C readline function

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
functionreadlinestackoverflow

Problem

I wrote this function for my next learning project (cfg parser). What do you think about it?

char readline(FILE *fp, char **line, size_t *size) {
    *size = 0;
    size_t cpos = ftell(fp);

    int c;
    while ((c = fgetc(fp)) != '\n' && c != EOF) (*size)++;
    if (*size == 0 && c == EOF) {
        *line = NULL;
        return 1;
    }

    *line = calloc(*size + 1, sizeof(char));
    if (*line == NULL)
        return -1;

    fseek(fp, cpos, SEEK_SET);
    if (fread(*line, 1, *size, fp) != *size)
        return -2;

    fgetc(fp); // Skip that newline

    return 0;
}


Can be used as follows:

FILE *fp = fopen("file.txt", "rb");
char *line = NULL;
size_t size = 0;
readline(fp, &line, &size);

Solution

-
sizeof(char) is guaranteed to be 1.

If you think that in future you might want to use another representation of characters (say, wchar_t), you'd have to modify code in more than one place. It is much safer to infer size from the variable, rather than from type:

*line = calloc(..., sizeof(**line));


This way there is a single modification point.

Similarly, you may want to use sizeof(**line) instead of literal 1 in fread.

-
fseek (and ftell)may fail with EBADF if the stream is not seekable. Since your function has no control over what kind of stream it is handled, it is advisable to test their return codes.

-
One could argue that reading 0 characters is in fact a success (a client can tell the difference by examining size), so differentiating return codes (0 and 1) is redundant.

It is highly recommended to use named macros (instead of magic numbers -1 and -2) for failure codes.

Code Snippets

*line = calloc(..., sizeof(**line));

Context

StackExchange Code Review Q#78798, answer score: 6

Revisions (0)

No revisions yet.