# Emulating C++ string input in C

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);
}
}

{
int ch = ' ';
while (ch = getc(stdin)) {

if (!isalpha(ch))
break;

}
}

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_print(&string);
String_free(&string);

getchar();
return 0;
}

• This approach consumes the trailing delimiter. Its value is lost. Is that truly intended? Jun 26 '18 at 11:33
• @chux That’s pretty standard behaviour for formatted input in C and C++. Jun 26 '18 at 11:36
• @KonradRudolph In C, it is not. scanf("%s", buf) does not consume trailing white-space after the populating buf. White-space is detected, yet returned to stdin. Similar for scanf("%d", &i) and others. Jun 26 '18 at 11:46
• @chux Fair point. I misremembered. Jun 26 '18 at 12:24

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;
}


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;

}
}


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)) {
}
}


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
}

{
string->size = 0;
int ch;
while ((ch = getc(stdin)) != EOF && !isspace(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_print(&string, stdout);
String_free(&string);
}

• thanks for the in deep review. i have to admit i didn't even expected to read much comments to this code because i considered the code to easy and short :-) I was quite wrong... One question i have hwat would be an better error handling? and why exit(1) not exit(-1) Jun 25 '18 at 18:34
• One nit to pick. I would change "if (!tmp)" to "if (tmp == NULL)", because tmp is not a logical (true/false) condition, it's a comparison to a special value, the NULL pointer. FALSE and NULL just happen to be implemented as 0 in C, but that's not necessarily universally true. Jun 25 '18 at 20:33
• @james/hoffmale: It's a standard C (and C++) idiom to test the validity of a pointer by leaving the comparison to 0 implicit. Remember that a literal 0 always means a null pointer (whether or not the machine uses all-bits-zero for its representation). NULL doesn't "just happen" to be implemented as 0 - the Standard insists that it must be 0, even on hardware where the zero address is valid. In those cases, the implementation is required to evaluate a pointer holding the zero address as a "true" value. Jun 26 '18 at 7:35
• Jun 26 '18 at 7:42
• Detail: "0 always means a null pointer" is amiss. 0 is always a constant int with value 0. Comparing that int to a pointer p, converts the int 0 to a null pointer. IAC, if (!tmp) is OK, yet as a minor point, I prefer if (tmp == NULL) as negations tend to be less clear than positive assertions - don't you not think is won't be otherwise ;-)? (Good review) Jun 26 '18 at 11:41

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

• 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

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) {