How to fix? Triggers exception when trying to free dynamic allocated memory

Question

This is my first time posting question, and I did try to find solution, but, even if I did found it I didn't recognize it.

So, as the title says, the problem is in this triggered exception "Exception thrown at 0x0F26372D (ucrtbased.dll) in lab10.exe: 0xC0000005: Access violation reading location 0xCCCCCCC4.

If there is a handler for this exception, the program may be safely continued.", which happens when I step into line -> free(word).

This did happen to me a few times when I was learning malloc, but I overlooked it - thinking there was some other problem. But now I see that I'am doing something wrong.

The point of the program is - writing the struct "word". I need to input sentence and "cut" it into words, and then every word put in struct together with size of letters in the word and ordinal number of the word.

#include <stdio.h>
#include <string.h>

struct word {
    char text_word[50];
    unsigned sizee; //number of letters of the word
    unsigned number; //ordinal number of the word
};

void cutting_sentence(struct word *p, char *sen) { //sen is sentence
    int size_sen, i, j;

    size_sen = strlen(sen) + 1; //size of sentence

    p = (struct word*)malloc(size_sen * sizeof(struct word)); 
    if (p == NULL) {
        printf("\nNot enaugh memory!");
        return 0;
    }

    strcpy(p[0].text_word, strtok(sen, " ,.!?")); 
    p[0].sizee = strlen(p[0].text_word);
    p[0].number = 1;

    printf("word:%s \t size:%u \t ordinal number of the word:%u\n", 
        p[0].text_word, p[0].sizee, p[0].number);

    for (i = p[0].sizee - 1, j = 1;i < size_sen;++i) {
        if (*(sen + i) == ' ' || *(sen + i) == '.' || *(sen + i) == ','
        || *(sen + i) == '?' || *(sen + i) == '!') {
            strcpy(p[j].text_word, strtok(NULL, " ,.!?"));
            p[j].sizee = strlen(p[j].text_word);
            p[j].number = j + 1;

            printf("word:%s \t size:%u \t ordinal number of the 
                        word:%u\n", p[j].text_word, p[j].sizee, p[j].number);

            j++;
        }
    }
}

int main() {
    char sentence[1024];
    struct word *word;

    printf("Sentence: ");
    gets(sentence);

    cutting_sentence(&word, sentence);

    free(word);  //here is exception triggered

    return 0;
}

Answer 1

You're changing the local value of the pointer argument passed, you need to change the memory at its target for the caller to discover the location of the allocated memory. Since you didn't do that, you're trying to free the local variable word which is stored on the stack of main().

First thing to fix is not to have a variable identical to the name of a type, that's just evil.

Then change the function prototype to pass a double pointer:

void cutting_sentence(struct word **p, char *sen);

And remember that where you were using p you now need to use *p or first assign a local (word *) with the address value contained there.

void cutting_sentence(struct word **p, char *sen) { //sen is sentence
    int size_sen, i, j;

    size_sen = strlen(sen) + 1; //size of sentence

    *p = (struct word*)malloc(size_sen * sizeof(struct word)); 
    if (*p == NULL) {
        printf("\nNot enaugh memory!");
        return; //void cannot return a value
    }

and so on changing every usage of p to *p.

and then

int main() {
    char sentence[1024];
    struct word *words;

    printf("Sentence: ");
    gets(sentence);

    cutting_sentence(&words, sentence);

    if (words != NULL)
       free(words);  //now valid

    return 0;
}

Answer 2

There are a few more issues than those previously discussed.

[As already pointed out] your first argument should be struct word **. But, a simpler way is to eliminate it and change the return type to struct word *. This allows the code within the function to be simpler (i.e. no double dereferencing of a pointer)

Although allocating as many word structs as there are characters in the input string will work, that is somewhat unusual.

A better way [more idiomatic, at least] to do this is to use realloc inside the loop.

In either case, the array size can be trimmed to only use what it needs via a final realloc.

I think that your loop that scans sen looking for delimiters is overly complex. Simply using strtok in the loop will give the same effect with less complexity.

Also, you're not conveying back a count of the number of word. One way is to add an extra element to the array that has a size of zero (e.g. an end-of-list marker)

---

Here's is a refactored version that should help:

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

struct word {
    char text_word[50];
    unsigned sizee;                     // number of letters of the word
    unsigned number;                    // ordinal number of the word
};

struct word *
cutting_sentence(char *sen)
{                                       // sen is sentence
    int curcnt = 0;
    int maxcnt = 0;
    char *token;
    struct word *word;
    struct word *words;

    while (1) {
        token = strtok(sen," ,.!?\n");
        if (token == NULL)
            break;
        sen = NULL;

        if (curcnt >= maxcnt) {
            maxcnt += 100;
            words = realloc(words,sizeof(struct word) * (maxcnt + 1));
        }

        word = &words[curcnt];
        strcpy(word->text_word,token);
        word->number = curcnt;
        word->sizee = strlen(token);

        ++curcnt;
    }

    words = realloc(words,sizeof(struct word) * (curcnt + 1));

    // set end-of-list
    word = &words[curcnt];
    word->sizee = 0;

    return words;
}

int
main()
{
    char sentence[1024];
    struct word *words;
    struct word *word;

    printf("Sentence: ");
    fflush(stdout);

    fgets(sentence,sizeof(sentence),stdin);

    words = cutting_sentence(sentence);

    for (word = words;  word->sizee != 0;  ++word)
        printf("main: number=%u sizee=%u text_word='%s'\n",
            word->number,word->sizee,word->text_word);

    free(words);

    return 0;
}

Answer 3

the following proposed code:

1. eliminates redundant code
2. properly checks for errors
3. properly outputs error message (and the text reason the system thinks an error occurred to stderr
4. performs the desired functionality
5. properly initializes the struct word pointer
6. properly updates the struct word pointer
7. changed int sizee to size_t sizee because the function: strlen() returns a size_t, not an int
8. changed int i to unsigned i because the declaration of the struct field number is declared as unsigned
9. documents why each header file is included
10. allocates an instance of the struct word for every character in the sentence This is 'overkill'. The most possible amount of words would be if every word was only a single character. So, immediately, the size of the allocated memory could be cut in half. A loop, counting the word separators would result in the correct amount of allocated memory. You could easily add that feature.
11. Note the way that the function: strtok() is used. I.E. initial call before loop, then following calls at end of loop

And now the proposed code:

#include <stdio.h>   // printf(), fgets(), NULL
#include <stdlib.h>  // exit(), EXIT_FAILURE, malloc(), free()
#include <string.h>  // strlen(),  strtok()


struct word 
{
    char text_word[50];
    size_t sizee; //number of letters of the word
    unsigned number; //ordinal number of the word
};

// notice the `**p` so can access the pointer in `main()` so it can be updated
void cutting_sentence(struct word **p, char *sen) 
{ //sen is sentence
    size_t size_sen = strlen(sen); //size of sentence
    struct word *wordptr = *p;

    wordptr = malloc(size_sen * sizeof(struct word)); 
    if ( !wordptr ) 
    {
        perror("malloc failed");
        exit( EXIT_FAILURE );
    }


    unsigned i = 0;
    char * token = strtok(sen, " ,.!?");
    while( token )
    {
        strcpy( wordptr[i].text_word, token ); 
        wordptr[i].sizee = strlen( token );
        wordptr[i].number = i;

        printf("word:%s\t Length:%lu]tordinal number of the word:%u\n", 
                wordptr[i].text_word, 
                wordptr[i].sizee, 
                wordptr[i].number);

        token = strtok( NULL, " ,.!?");
        i++;
    }
}

int main( void ) 
{
    char sentence[1024];
    struct word *wordArray = NULL;

    printf("Sentence: ");
    if( !fgets(sentence, sizeof( sentence ), stdin ) )
    {
        perror( "fgets failed" );
        exit( EXIT_FAILURE );
    }

    // remove trailing new line
    sentence[ strcspn( sentence, "\n") ]  = '\0';
    cutting_sentence(&wordArray, sentence);

    free( wordArray );  //here is exception triggered

    return 0;
}

A typical run of the code results in:

Sentence: hello my friend
word:hello   Length:5   ordinal number of the word:0
word:my  Length:2   ordinal number of the word:1
word:friend  Length:6   ordinal number of the word:2

Notice that 'short' words result in an uneven output. You might want to correct that.