Emulating C++ string input in C

Question

I did the following exercise:

Write a C program that does the equivalent of C++ string s; cin>>s;; that is, define an input operation that reads an arbitrarily long sequence of whitespace-terminated characters into a zero terminated array of chars.

I wonder if it's good code. What could be improved?

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

struct String {
    char* signs;
    size_t size;
    size_t capacity;
};

void String_allocate_space(char **c, size_t *capacity)
{
    if (*capacity == 0) {   // allocate the first time
        *capacity = 1;
        *c = malloc(sizeof(**c) * ((*capacity)));
    }
    else {
        *capacity *= 2;     // double the new capacity
        *c = realloc(*c, sizeof(**c) * (*capacity));
    }
    if (*c == NULL)
        exit(-1);
}

void add_character(struct String* string, int ch)
{
    if (string->size == string->capacity) {     // if current letter sz = capacity
        String_allocate_space(&string->signs, &string->capacity);
    }
    string->signs[string->size++] = ch;    // append the sign in the array
}

void String_read(struct String* string)
{
    int ch = ' ';
    while (ch = getc(stdin)) {

        if (!isalpha(ch))
            break;

        add_character(string, ch);
    }
    add_character(string, '\0');
}

void String_print(struct String* string)
{
    printf("%s", string->signs);
}

void String_free(struct String* string)
{
    int i = 0;
    for (i = 0; i < string->capacity; ++i) {
        free(string[i].signs);
        string[i].signs = NULL;
    }
}

int main()
{
    struct String string = { 0 };

    String_read(&string);
    String_print(&string);
    String_free(&string);

    getchar();
    return 0;
}

Answer 1

Don't do this:

*c = realloc(*c, sizeof(**c) * (*capacity));

Once you have error handling that's more sophisticated than exit(1), this will become a liability. You need a temporary:

char *tmp = realloc(*c, new_capacity);
if (!tmp) {
    /* error handling - c is still valid */
    /* ... */
}
*c = tmp;
*capacity = new_capacity;

---

If you always initialize the data pointer to start as a null pointer, you don't need to use malloc() instead of realloc(). String_allocate_space would be easier to write if it accepts a pointer to a struct String; that makes it closer to the object-oriented version:

void String_init(struct String* string)
{
    string->data = NULL;
    string->size = string->capacity = 0;
}

void String_allocate_space(struct String* string)
{
    size_t new_capacity = string->capacity ? 2 * string->capacity : 16;
    char *tmp = realloc(string->data, new_capacity);
    if (!tmp) {
        /* error handling - c is still valid */
        exit(1);                /* TODO: improve error reporting */
    }
    string->data = tmp;
    string->capacity = new_capacity;
}

I've also incorporated a change above to start with a larger initial size (16) instead of 1. That lets us skip the first 4 reallocations for free.

---

The read() method has a flaw that will become apparent when you try to read another value into a string - unlike std::string, reading with >> will append to it, instead of replacing it. We need to reset size at the beginning:

void String_read(struct String* string)
{
    string->size = 0;
    int ch;
    while (ch = getc(stdin)) {

        if (!isalpha(ch))
            break;

        add_character(string, ch);
    }
    add_character(string, '\0');
}

Also, the logic is slightly wrong - we want to finish when we see a space, rather than any non-alpha (which could be digits or punctuation characters). (Well done for remembering that getch() returns int rather than char - that's one common error avoided).

void String_read(struct String* string)
{
    string->size = 0;
    int ch;
    while ((ch = getc(stdin)) != EOF && !isspace(ch)) {
        add_character(string, (char)ch);
    }
    add_character(string, '\0');
}

---

When we free the string, we don't need a loop. Instead, we have a single free(). It's a good idea to reset the size and capacity so that the string object is consistent - it can be used again and/or freed again without harm:

void String_free(struct String* string)
{
    free(string->data);
    string->data = NULL;
    string->size = string->capacity = 0;
}

This is an important concept in object-oriented programming - objects have invariants that they guarantee are true at the start and end of every (public) operation. In this case, the invariants are that

• data points to valid storage of at least capacity if capacity > 0 and is a null pointer otherwise.
• size is not greater than capacity.

---

We can improve the printing so that it outputs any embedded NUL characters, just like C++ strings do:

void String_print(struct String* string)
{
    if (fwrite(string->data, 1, string->size, stdout) != string->size) {
        exit(1);                /* TODO: improve error reporting */
    }
}

Note that this will now print the trailing NUL we added. We no longer need that to mark the end of string, so we can remove that line.

---

Modified code

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

struct String {
    char* data;
    size_t size;
    size_t capacity;
};

void String_init(struct String* string)
{
    string->data = NULL;
    string->size = string->capacity = 0;
}

void String_allocate_space(struct String* string)
{
    size_t new_capacity = string->capacity ? 2 * string->capacity : 1;
    char *tmp = realloc(string->data, new_capacity);
    if (!tmp) {
        /* error handling - c is still valid */
        exit(1);                /* TODO: improve error reporting */
    }
    string->data = tmp;
    string->capacity = new_capacity;
}

void add_character(struct String* string, char ch)
{
    if (string->size == string->capacity) {     // if current letter exceeds capacity
        String_allocate_space(string);
    }
    string->data[string->size++] = ch;    // append it
}

void String_read(struct String* string)
{
    string->size = 0;
    int ch;
    while ((ch = getc(stdin)) != EOF && !isspace(ch)) {
        add_character(string, (char)ch);
    }
}

void String_print(struct String *restrict string, FILE *restrict stream)
{
    if (fwrite(string->data, 1, string->size, stream) != string->size) {
        exit(1);                /* TODO: improve error reporting */
    }
}

void String_free(struct String* string)
{
    free(string->data);
    string->data = NULL;
    string->size = string->capacity = 0;
}

int main()
{
    struct String string;

    String_init(&string);
    String_read(&string);
    String_print(&string, stdout);
    String_free(&string);
}

Answer 2

Since @TobySpeight has already posted a wonderful answer, I'm not going to repeat what he has already posted.

Just some small additional notes:

• String_allocate_space allocates 1 chars worth of memory if the capacity is 0. This is barely enough to hold the terminating '\0' character, but nothing more. Maybe increase the default minimal allocation size a bit to be meaningful?

• Also, regarding naming: If I see a function called String_free, I'd expect a function String_alloc that allocates and creates the String object in a well-defined state. (And consequently, String_free should then deallocate that String correctly.)

Also, this might just be me, but from the task description I'd expect the solution to be a char *read_input(void) function. While the String "class" is nice, it seems like a bit of over-engineering for the task at hand.

For comparison, look at this solution:

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

#define INITIAL_INPUT_CAPACITY 4

char *read_input(void) {
    size_t capacity = INITIAL_INPUT_CAPACITY;
    size_t size = 0;
    char *str = malloc(capacity * sizeof(char));
    int input;

    while((input = getc(stdin)) != EOF) {
        if(isspace(input)) break;

        str[size++] = (char)input;

        if(size == capacity) {
            capacity *= 2;
            char *temp = realloc(str, capacity * sizeof(char));

            if(temp == NULL) {
                exit(-1);
            }

            str = temp;
        }
    }

    str[size++] = '\0';

    return str;
}

int main(void) {
    char *input = read_input();
    puts(input);
    free(input);

    getchar();
    return 0;
}