Problems with dynamic memory allocation for structs in C

Question

I'm given an unknown amount of "wide symbols". The text is formatted as sentences, which i have to add to struct "Text".

These are my structs:

struct Sentence {
    wchar_t *sentence;
    int amount_of_symbols;
};

struct Text {
    struct Sentence *sentences;
    int amount_of_sentences;
};

I dynamically allocate memory for array of "Sentence" structs and add them. This is my input code:

int amount_of_sentences = 0;
struct Sentence *sentences = (struct Sentence *) malloc(amount_of_sentences * sizeof(struct Sentence));

struct Text text = {sentences, amount_of_sentences};

wchar_t symbol;
int buffer_size = 0;
wchar_t *buffer = (wchar_t *) malloc(buffer_size * sizeof(wchar_t));

bool sentence_begun = true;

while (true) {
    symbol = getwchar();

    if (symbol == '\n')
        break;

    if (sentence_begun && symbol == ' ') {
        sentence_begun = false;
        continue;
    }

    buffer = (wchar_t *) realloc(buffer, (++buffer_size) * sizeof(wchar_t));
    buffer[buffer_size - 1] = symbol;

    if (symbol == '.') {
        buffer[buffer_size] = '\0';

        text.amount_of_sentences++;
        text.sentences = (struct Sentence *) realloc(text.sentences, text.amount_of_sentences * sizeof(struct Sentence));
        text.sentences[text.amount_of_sentences - 1].amount_of_symbols = buffer_size;
        text.sentences[text.amount_of_sentences - 1].sentence = (wchar_t *) malloc(buffer_size * sizeof(wchar_t));
        text.sentences[text.amount_of_sentences - 1].sentence = buffer;

        buffer_size = 0;
        buffer = (wchar_t *) realloc(buffer, buffer_size * sizeof(wchar_t));
        sentence_begun = true;
    }
}

Everything seems to be fine, but as soon as i try to output all my sentences, not all of them are shown and some of the are repeated.

This is my output code:

for (int i = 0; i < text.amount_of_sentences; i++) {
    wprintf(L"%ls\n", text.sentences[i].sentence);
}

Example of input-output:

adjsand. asdad.a.a. aaaa. adsa.


a.

adsa.
adsa.

What can be wrong with this code and what should i change?

Could you show how `text` and `sentence_begun` are declared and initialized? — Schwern, Dec 12 '18 at 10:57
@Schwern, int amount_of_sentences = 0; struct Sentence *sentences = (struct Sentence *) malloc(amount_of_sentences * sizeof(struct Sentence)); struct Text text = {sentences, amount_of_sentences}; bool sentence_begun = true; — Никита Бабенко, Dec 12 '18 at 10:59
[Please edit your question](https://stackoverflow.com/posts/53741132/edit) with that so the code example is more complete. — Schwern, Dec 12 '18 at 11:07

score 0 · Answer 1 · answered Dec 12 '18 at 10:59

0

First, your buffer is 1 too small and doesn't account for the terminating '\0'. At the top of your program, do:

int buffer_size = 1;
wchar_t *buffer = (wchar_t *) malloc(buffer_size * sizeof(wchar_t));
*buffer= '\0';

But the real problem is in:

    text.sentences[text.amount_of_sentences - 1].sentence =
                (wchar_t *) malloc(buffer_size * sizeof(wchar_t));
    text.sentences[text.amount_of_sentences - 1].sentence = buffer;

    buffer_size = 0;
    buffer = (wchar_t *) realloc(buffer, buffer_size * sizeof(wchar_t));

You allocate memory for the sentence, but then you overwrite that pointer with the buffer pointer. Next you reset the buffer size and reallocate the buffer.

The assignment does not copy the buffer data. To do that, do:

    strcpy(text.sentences[text.amount_of_sentences - 1].sentence, buffer);

and here too, do:

buffer_size = 1;
buffer = (wchar_t *) realloc(buffer, buffer_size * sizeof(wchar_t));
*buffer= '\0';

answered Dec 12 '18 at 10:59

Paul Ogilvie

25,048
4
23
41

It didn't help. The output is similar to the first one. – Никита Бабенко Dec 12 '18 at 11:12
I forgot... `buffer[buffer_size] = '\0';` should be `buffer[buffer_size-1] = '\0';` If that doesn't fix it (and I can't find anytrhing more wrong with the code), you must take the program in a debugger and step through it, checking at each step if it still OK. – Paul Ogilvie Dec 12 '18 at 11:30
Also: `amount_of_symbols = buffer_size -1;` because the last char is the `'\0'` – Paul Ogilvie Dec 12 '18 at 11:33
And: `buffer[buffer_size - 2] = symbol;` for the same reason. – Paul Ogilvie Dec 12 '18 at 11:35
@PaulOgilvie Is `strcpy` safe with `wchar_t *`? – Schwern Dec 12 '18 at 12:12

Schwern · Accepted Answer · 2018-12-12T12:12:57.777

The problem is here.

    text.sentences[text.amount_of_sentences - 1].sentence = (wchar_t *) malloc(buffer_size * sizeof(wchar_t));
    text.sentences[text.amount_of_sentences - 1].sentence = buffer;

    buffer_size = 0;
    buffer = (wchar_t *) realloc(buffer, buffer_size * sizeof(wchar_t));

You allocate a fresh sentence with malloc and then overwrite it with buffer. This will leak memory.

Then you assign buffer to text.sentences[text.amount_of_sentences - 1].sentence and then free that memory by reallocating buffer.

From the C standard...

The realloc function deallocates the old object pointed to by ptr and returns a pointer to a new object that has the size specified by size.

text.sentences[text.amount_of_sentences - 1].sentence winds up pointing to freed memory. This will lead to undefined behavior.

Instead, point to buffer and allocate a new buffer.

    text.sentences[text.amount_of_sentences - 1].sentence = buffer;

    buffer_size = 0;
    buffer = malloc(buffer_size * sizeof(wchar_t));

Some other notes...

As Paul noted, you need to allocate an extra byte for the null byte.

There's no need to cast the result of malloc or realloc.

It's simpler, faster, and less bug prone to allocate a large buffer on the stack to read input into (growing it if necessary). Then copy the contents to appropriately sized memory.

I went ahead and coded up an improved version to illustrate. If this is homework, please don't hand this in.

@НикитаБабенко You're welcome. I went ahead and coded up a version with the improvements I was talking about. https://gist.github.com/schwern/75f4bf42baf36042a5416cf782438438 Use it for illustration, but if this is homework please don't turn it in as your own work. — Schwern, Dec 12 '18 at 12:05

Problems with dynamic memory allocation for structs in C

2 Answers2