Why does the memory not allocate?

Question

My assignment is to allow the user to enter any input and print the occurrences of letters and words. We also have to print out how many one letter, two, three, etc.. letter words are in the string.

The word function has an access violation when dealing the array of pointers. It looks like the malloc() function didn't properly allocate memory to my array of pointers and I'm not sure I did the coding right.

I actually tried allocating memory to an index of the array, word[0], and that index had memory that was properly allocated, but when I use a loop it never works, when I hover over the array and check each index it says "Bad PTR".

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


void findLetters(char *ptr);
void findWords(char *point);


int main()
{
    char textStream[100]; //up to 98 characters and '\n\ and '\0'

    printf("enter some text\n");
    if (fgets(textStream, sizeof (textStream), stdin)) //input up to 99 characters
    {
        findLetters(textStream);
        findWords(textStream);
    }
    else
    {
        printf("fgets failed\n");
    }

    return 0;
}

void findLetters(char *ptr) //find occurences of all letters
{ /*Works fine*/ }

void findWords(char *point)
{
    int i = 0;
    int k = 0;
    int count = 0;
    int j = 0;
    int space = 0;
    int c = 0;
    char *word[50];
    char word1[50][100];
    char* delim = "{ } . , ( ) ";

    for (i = 0; i< sizeof(point); i++) //counts # of spaces between words
    {
        if ((point[i] == ' ') || (point[i] == ',') || (point[i] == '.'))
        {
            space++;
        }
    }
    char *words = strtok(point, delim);
    for(;k <= space; k++)
    {
        word[k] = malloc((words+1) * sizeof(*words));
    }

        while (words != NULL)
        {
            printf("%s\n",words);
            strcpy(words, word[j++]);
            words = strtok(NULL, delim);
        }

    free(words);
}

What's wrong in my code?

Answer 1

 while (words != NULL)
 {
      printf("%s\n",words);
      strcpy(words, word[j++]);
      words = strtok(NULL, delim);
 }

free(words);

Think what this code is doing; it loops until words == NULL, then tries to free (words), which, if the loop has terminated, is NULL. So, you're trying to free a NULL pointer.

BTW. You don't need to free strtok's return value: Do I need to free the strtok resulting string?

EDIT: The solution is this:

1. for (i = 0; i< sizeof(point); i++) should be for (i = 0; i< strlen(point); i++) - sizeof(char*) is not the string's length, but the size of a char pointer on your system (4 or 8).

2. Replace everything after that for loop above with:

   char *words = strtok(point, delim);
   for (; k <= space && words != NULL; k++)
   {
       if (k >= 50)    //size of the word array
       {
           puts ("Too many words!");
           return;
       }      
   
       word[k] = malloc(strlen(words) + 1);
   
       strcpy(word[k], words);
       words = strtok(NULL, delim);
   }
   
   for (int i = 0; i < k; i++)
       free(word[i]);
   

That code is from Cool Guy's answer, except he had a bug there - the code was incrementing k twice.

Note that this code is pretty pointless as it is, it just allocates some memory, copies some stuff there, and frees that memory without doing anything, but I assume you want to do something else in the findWords function after that.

Answer 2

strtok replaces all delimiters with '\0' (= it modifies your input string).

So if you want to build an array containing pointers to all the words in your input array you can simply write the following:

void findWords(char *point)
{
    int count = 0;
    char *word[50];
    char* delim = "{ } . , ( ) ";
    char *words = strtok(point, delim);

    count = 0;
    while (words != NULL)
    {
        word[count] = words;
        count++;

        printf("%s\n",words);
        words = strtok(NULL, delim);

        if (count >= 50)   // word is limited to char *word[50] !!!
        {
             printf("too much words!\n");
             break;
        }
    }

    /*
    ** now count contains the number of words and
    ** word[0 .. (count - 1)] contains the words
    */
}

There is no need to allocate memory.

Running this little test

char test1[] = "hallo test, 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50";
char test2[] = "hallo test (2)";

findWords(test1);
findWords(test2);

shows the following (break point before leaving function findWords):

Within function findWords the contents of word are correct.

Answer 3

Try replacing

for(;k <= space; k++)
{
    word[k] = malloc((words+1) * sizeof(*words));
}

    while (words != NULL)
    {
        printf("%s\n",words);
        strcpy(words, word[j++]);
        words = strtok(NULL, delim);
    }

free(words);

with

for(;k <= space && words != NULL; k++)
{
    //word[k] = malloc((words+1) * sizeof(*words)); //Doesn't do what you think; You need strlen
    word[k] = malloc( strlen(words) + 1); //+1 for the NUL-terminator

    printf("%s\n",words);
    strcpy(word[k], words); //Arguments were mixed up. You want the opposite
    words = strtok(NULL, delim);
}

for(int i = 0; i < k; i++)
    free(word[i]);       //Free each index as you've allocated each of them not `words`

Also,

for (i = 0; i< sizeof(point); i++)

should be

for (i = 0; i< strlen(point); i++)

or better

int len = strlen(point);
for (i = 0; i < len; i++)

because sizeof(point) gives the size of a char* which is not want you want. So, use strlen.