Thread: memory and strtok function!!

Hi!
the program below is supposed to accept a sentence, and then print out the words in that sentence. the program works fine, but in the end it gives me an error specifying that memory couldn't be read.
do I dismanage my memory here?

Code:

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
int main(int arg, char* argv[]){
             char *sentence,words[5][200];
             int i = 0;
             sentence = (char*) malloc(sizeof(char) * 200);
             printf("Please enter your sentence(s):\n");
             gets(sentence);
             strcpy(words[i] , strtok(sentence," "));
             while (words[i]){
                   puts(words[i]);
                   i++;
                   strcpy(words[i] , strtok(NULL," "));
                   }
             system("PAUSE");
             return 0;

I see a lot of suspect code here. It's completely broken. I'd start over.

Code:

gets(sentence);
strcpy(words[i] , strtok(sentence," "));

The problem with statements like these is that it is all too easy to type too much text and write off the end of the array, which, to borrow your phrasing "mismanages memory." This evokes undefined behavior.

Also, strcpy gives no guarantee it will handle NULL safely, a pointer strtok can return on failure.

If we give the user the benefit of the doubt and assume he's cooperative, then you definitely run into an issue here:

Code:

while (words[i]){

When I ran your program through with my debugger, this was where execution broke. This condition doesn't work: it implicitly compares a pointer to zero. Bounds checking for i is the correct way to make sure you fill in all the words you have space for. Do bounds checking for all your arrays from now on so that you don't waltz off the end of them, or execute code more times than you should. If you do bounds checking you should have a completely easier time with arrays.

Well, we didn't cover fgets so far in class, and also it's my first time using strtok..
The problem is that I want to sotre every word of the sentence in a string, that why I used that TWO dimensional array.
Ok but why this is not working

Code:

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
int main(int arg, char* argv[]){
             char *sentence,words[200];
             int i = 0;
             sentence = (char*) malloc(sizeof(char) * 200);
             printf("Please enter your sentence(s):\n");
             gets(sentence);
             strcpy(words , strtok(sentence," "));
             while (words){
                   puts(words);
                   strcpy(words,strtok(NULL," "));
                   }
             system("PAUSE");
             return 0;

compared to the code in this webpage.
http://www.cplusplus.com/reference/c...ng/strtok.html

It's the same as before. You can't control the loop with the words pointer because it doesn't make sense to do it that way in this program or the snippet you just posted.

I believe your snippet should be:

Code:

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

int main(void)
{
   char sentence[200];
   char * token;
   
   printf("Enter a sentence\n");
   gets(sentence); 
   /** throwing caution to the wind... **/
   
   token = strtok(sentence, " ");
   while (token != NULL) {
      puts(token);
      token = strtok(NULL, " ");
   }
   return 0;
}

A proper way to do this would be to look at strtok's return value, and if it's something other than NULL, store it or print it. But you take it for granted that strtok will always return a word you can copy or that anything you do in this loop will have an effect on word so that the loop will break on time.

It's really not that difficult, you've just got to be sure your loops have proper test conditions.
For example, in your earlier program, there might be more words in the sentence than there are spaces. Or strtok might return NULL so you have no substring to copy. Without handling either of these, you cannot fix your earlier program.