Read arbitrary length strings in C

Question

This is a classic thing to want to do since it's in C that one doesn't have the std::string of C++ and input of Python. I believe that the best thing to do is to read the input stream character by character, since scanf is unsafe and fgets requires an expected size to be passed, things which I do not want to do.

I've read other answers in this site, but they seem to overcomplicate things too much, maybe the following code fails in borderline cases and I'm just ignorant, but as far as I've tried, this works fine. Even with piping on bash.

The reason i do realloc(str, buf - 1) is to be able to store up to SIZE_MAX since, when an overflow occurs, buf will equal 0 and 0 - 1 in a size_t will be equal to SIZE_MAX.

So I tried the following approach:

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

int main(void)
{
        char *str = malloc(2);

        if (!str) {
                printf("Cannot allocate string.\n");
                return 1;
        }

        int c;
        char *reallocStr;
        size_t len = 0;
        size_t buf = 2;

        printf("Enter some text: ");

        while ((c = getchar()) != EOF && c != '\n') {
                if (len + 1 == buf) {
                        buf *= 2;
                        reallocStr = realloc(str, buf - 1);

                        if (!reallocStr) {
                                printf("Cannot reallocate string.\n");
                                free(str);
                                return 1;
                        }

                        str = reallocStr;
                }

                str[len++] = c;
        }

        str[len] = '\0';

        printf("You entered: %s\n", str);

        free(str);

        return 0;
}

Answer 1

        size_t buf = 2;

This seems exceptionally small. Rather than starting with 2, I'd start with, e.g. 500. Then at the end, after you add the null byte to terminate the string, do

        reallocStr = realloc(str, len + 1);
        if (!reallocStr) {
                printf("Cannot reallocate string.\n");
                free(str);
                exit(1);
        }

Also, I would exit rather than return. Then you can extract the realloc step into a separate method.

        str = realloc_or_die(str, len + 1);

with

char * realloc_or_die(char *original, int length) {
        char *replacement = realloc(original, length);
        if (replacement) {
                return replacement;
        }

        printf("Cannot reallocate string.\n");
        free(original);
        exit(1);
}

You're writing C. The compiler should be able to inline that function call if it takes too much time. Let the compiler do the repetitive parts. That's why we have computers.

You can also use this to replace the malloc call by calling realloc_or_die(NULL, buf);.

This will also reduce your levels of indent. As Linus Torvalds has proclaimed, "...if you need more than 3 levels of indentation, you’re screwed anyway, and should fix your program." Since Linus is one of the few people who prefers eight-column tabs for indent, you should keep with him if you do as well.

Many of the rest of us find four-column indent plenty sufficient and we're often willing to allow greater nesting. (In HTML, I find two-column indent to be plenty, precisely because the need for nesting is greater.) But if you're using the wider indent, you should keep the rules that go along with it.

Answer 2

Extract reusable code into functions

I feel that this code should really be its own function, along the lines of fgets.

Limits on much to read from stdin at maximum

Also, as it is, your code is exploitable by putting in extremely large strings as input. Though you consider SIZE_MAX to be the limit, this is unreasonably large. Making this a function would allow a bunch of customizations for users of your code.

Make memory management the responsibility of the caller

In C, it is a very common pattern for the caller of a function which needs to work on some memory to have to pass in a pointer to the memory it wants the function to use.

Rolling it all up

#include <stddef.h>
#include <stdint.h>
#include <stdlib.h>
#include <stdio.h>

size_t read_str_till_newline(FILE* source, char** buffer, const size_t initial_size, size_t max_size) {
    if (NULL == source || NULL == buffer || NULL == *buffer) {
        return 0;
    }
    if (max_size == 0) {
        max_size = SIZE_MAX;
    }

    int read_char;
    size_t read_len = 0;
    size_t current_size = initial_size;
    while((read_char = fgetc(source)) != EOF && // not reached end of input
          !(read_char == '\n' || read_char == '\r' || read_char == '\0') && // not a newline or NUL
          read_len < max_size) { // not reading too much
        if (read_len + 1 == current_size) {
            current_size *= 2;
            char* new_buffer = realloc(*buffer, current_size);
            if (NULL == new_buffer) {
                fprintf(stderr, "Realloc failed for reallocating read buffer: start size = %zu, new size = %zu, stopping reading further at %zu read chars\n", current_size / 2, current_size, read_len);
                return read_len;
            }
            *buffer = new_buffer;
        }
        *buffer[read_len++] = read_char;
    }
    *buffer[read_len] = '\0';
    return read_len;
}

int main(void) {
    char* buffer = calloc(2, 1);
    printf("Enter some text: ");
    size_t read_chars = read_str_till_newline(stdin, &buffer, 2, 8);
    printf("Read %zu chars, string was: %s\n", read_chars, buffer);
    free(buffer);
}

Answer 3

Reading one character at a time is slow

I believe that the best thing to do is to read the input stream character by character, since scanf() is unsafe and fgets() requires an expected size to be passed,

Reading one character at a time is slow. Even if getchar() reads data efficiently into a buffer behind the scenes, you still pay the overhead of the function call for each character.

fgets() doesn't require the expected size, just the maximum size. You know that the unused size of the buffer is buf - len, so you can pass that:

char *result = fgets(str, buf - len, stdin);
if (!result) {
    /* handle read error */
}

The problem is determining how much fgets() read in. You can use strlen(str + len) for that. This looks worse since now it has to go over the data again, but it might be worth it because in the end it's only \$O(\log N)\$ function calls, where \$N\$ is the length of the string. The remaining thing is checking that there is a newline at the end of the data read in so far; if so you read the whole line, otherwise you need to reallocate and call fgets() again.

Consider using getline()

If you are on a POSIX-compliant platform, you can use the function getline(), which does exactly what you want. However, it might limit the portability of your program.

Answer 4

Bug

After realloc(str, buf - 1), allocation is potentially one-too-small for the final str[len] = '\0';.

Instead:

  size_t buf = 1;
  char *str = malloc(buf);
  ...
    
    buf = buf * 2 + 1;
    reallocStr = realloc(str, buf);

Not useful enough

OP has "other answers in this site, but they seem to overcomplicate things too much", yet this code does not distinguish from reading a line of only a '\n' from immediately encountering end-of-file. This distinction is important for the caller. fgets() returns EOF when end-of-file immediately occurs.

If an input error occurs after reading some text, calling code has no hint that anything unusual happened. Common C practice is to return an indication of a read error. fgets() returns EOF any time an input error occurs.

Why 2?

Code could have started at char *str = malloc(1); (for minimality) or char *str = malloc(BUFSIZ); (see <stdio.h>) to reduce repetitive malloc(). 2 is unsatisfactory.

Hackable

The goal of "Read arbitrary length strings in C" allows the user to consume enormous resources before memory exhaustion. I do like the idea of allowing very long input strings, yet, IMO, robust still has an upper bound imposed by the program to defend itself against nefarious users. Caller could still call with sz == SIZE_MAX is "unlimited" input desired.

Perhaps as:

/*
 * Read input until \n, end-of-file or input error.
 * Save up to sz-1 non-'\n' characters.
 * If sz > 0, append a \0.
 *
 * With a non-NULL length_ptr:
 *   Set *length_ptr to the number of characters saved.
 *   This does not include any \n not the appended \0.
 *
 * Return
 * 1: Read 1 or more characters without input error and enough buffer space
 *    to save all non-'\n' and the appended \0.
 * 0: Otherwise no characters read due to end-of-file.
 * -1: Otherwise an input error occurred.
 * -2: Otherwise insufficient buffer space.
 */

int my_getline2(size_t sz, char dest[sz], size_t *length_ptr);