0

I've been trying to figure out what the problem is for hours and can't get it right. Here is the code, which is of course a lot longer but I've reduced it to the problem itself.

#define BUFFER_SIZE 60
char *str;

void readText() {
    char read_char;
    int i = 0;
    str = (char *) calloc(BUFFER_SIZE, sizeof(char));
    while ((read_char = getchar()) != EOF) { /* user hits ctrl+d */
        *(str+i) = read_char;
        i++;
        if (i % BUFFER_SIZE == 0) {
            str = (char *) realloc(str, BUFFER_SIZE * sizeof(char));
        }
    }
    textSize = i;
    /* Here I print the text... same error printing or not printing */
    free(str);
}

}

I only get the error when the input text exceeds the buffer size.

(edited: if (i % BUFFER_SIZE == 0) so it makes it every time it get to 60, but the error is the same )

Thanks

Adrian Laufer
  • 35
  • 6
  • 3
    Buffer overflow leads to *undefined behavior*. Don't let it happen. – Some programmer dude May 02 '21 at 08:56
  • By the way, the [`getchar`](https://en.cppreference.com/w/c/io/getchar) functions returns an **`int`**, which is rather important for the comparison against the `int` value `EOF`. – Some programmer dude May 02 '21 at 08:57
  • And on another note, for any pointer or array `p` and index `i` the expression `*(p + i)` is *exactly* equal to `p[i]`. The latter (`p[i]`) is usually easier to read and understand, and less to write. – Some programmer dude May 02 '21 at 08:58
  • 1
    Look at the realloc vs the original `calloc`. Is there *any* difference in the amount of memory allocated *at all* ? Um... no. And then you promptly walk off the end an breach that buffer to invoke UB. Equally bad, even if you fixed the sizing there, you need a dynamic *capacity* to be updated, not a fixed constant buffer size. – WhozCraig May 02 '21 at 09:00
  • Then we have the `realloc` call itself, which doesn't reallocate anything. You just allocate the same size over and over. Just like `malloc` its size argument is the size in bytes to allocate. – Some programmer dude May 02 '21 at 09:00
  • 1
    And lastly, in C you [shouldn't really cast the result of `malloc`, `calloc` or `realloc`](https://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc/605858). – Some programmer dude May 02 '21 at 09:01

1 Answers1

1

That's because you are reallocating with the same size, you need to use the new size when the string grows up:

while ((read_char = getchar()) != EOF) { /* user hits ctrl+d */
    if (i >= BUFFER_SIZE) {
        str = realloc(str, i);
    }
    *(str+i) = read_char;
    i++;
}

Also, it seems that you forget to set the trailing NUL, you need it in order to build a valid (printable) string, switch to

    if (i >= BUFFER_SIZE - 1) {

and

str[i] = '\0';

after the while loop.

Finally, prefer

size_t i = 0; // or better yet size_t len = 0;

over

int i = 0;

to pass the size to realloc and friends.

David Ranieri
  • 39,972
  • 7
  • 52
  • 94