Stack Smashing and using malloc

Question

I'm making a program that counts the number of words contained within a file. My code works for certain test cases with files that have less than a certain amount of words/characters...But when I test it with, let's say, a word like:

"loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong", (this is not random--this is an actual test case I'm required to check), it gives me this error:

*** stack smashing detected ***: ./wcount.out terminated
Abort (core dumped)

I know what the error means and that I have to implement some sort of malloc line of code to be able to allocate the right amount of memory, but I can't figure out where in my function to put it or how to do it:

int NumberOfWords(char* argv[1]) {
  FILE* inFile = NULL;
  char temp_word[20]; <----------------------I think this is the problem

  int num_words_in_file;
  int words_read = 0;

  inFile = fopen(argv[1], "r");

  while (!feof(inFile)) {
    fscanf(inFile, "%s", temp_word);
    words_read++;
  }
  num_words_in_file = words_read;

  printf("There are %d word(s).\n", num_words_in_file - 1);
  fclose(inFile);
  return num_words_in_file;

}

Not sure if you wanted to tag [tag:c++] or what, but this is completely unrelated to [tag:c#]. — Camilo Terevinto, Nov 23 '17 at 21:48
So you're reading a string of unknown length into 20 bytes and wondering why why are stomping on the stack? Use more than 20? Read 20 (probably 19) chars at a time. Many possible approaches. — John3136, Nov 23 '17 at 21:53
There are several errors in the code. Not testing `inFile` after `fopen`. Please see [Why is “while ( !feof (file) )” always wrong?](http://stackoverflow.com/questions/5431941/why-is-while-feof-file-always-wrong) `char temp_word[20]` cannot hold the 247-length string you attempt to read with the unrestricted `fscanf(inFile, "%s", temp_word)` and so on. — Weather Vane, Nov 23 '17 at 21:56
If you are intending to count the number of words in a file, then throwing `"loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong"` at it is stupid. There are very few words with 20 or more characters, so increase the array length to say 100, and restrict the input length. — Weather Vane, Nov 23 '17 at 22:00
Re the edit: is the "actual test case" you are required to input supposed to be a valid word, or a test case that you should reject? — Weather Vane, Nov 23 '17 at 22:03
It should be a valid test case that should print, "There are 1 word(s)." Regardless of the length/ridiculousness of the word, it's still 1 read "word". — Jihane Eter, Nov 23 '17 at 22:07
All I want to do is use malloc to not worry about how large a word will be and continue with the program to count the number of words — Jihane Eter, Nov 23 '17 at 22:08
Even with malloc you need to know the length of the word BEFORE. — Yunnosch, Nov 23 '17 at 22:09

score 0 · Answer 1 · answered Nov 23 '17 at 22:08

As you've correctly identified by rendering your source code invalid (future tip: /* put your arrows in comments */), the problem is that temp_word only has enough room for 20 characters (one of which must be a terminal null character).

In addition, you should check the return value of fopen. I'll leave that as an exercise for you. I've answered this question in other questions (such as this one), but I don't think just shoving code into your face will help you.

In this case, I think it may pay to better analyse the problem you have, to see if you actually need to store words to count them. As we define a word (the kind read by scanf("%s", ...) as a sequence of non-whitespace characters followed by a sequence of (zero or more) whitespace characters, we can see that such a counting program as yours needs to follow the following procedure:

Read as much whitespace as possible
Read as much non-whitespace as possible
Increment the "word" counter if all was successful

You don't need to store the non-whitespace any more than you do the whitespace, because once you've read it you'll never revisit it. Thus you could write this as two loops embedded into one: one loop which reads as much whitespace as possible, another which reads non-whitespace, followed by your incrementation and then the outer loop repeats the whole lot... until EOF is reached...

This will be best achieved using the %*s directive, which tells scanf-related functions not to try to store the word. For example:

size_t word_count = 0;
do {
     fscanf(inFile, "%*s");
} while (!feof(inFile) && ++word_count);

score 0 · Answer 2 · answered Nov 23 '17 at 22:10

You are limited by the size of your array. A simple solution would be to increase the size of your array. But you are always susceptible to stack smashing if someone enters a long word.

A word is delimited by spaces.

You can simply store a counter variable initialized to zero, and a variable that records the current char that you are looking at. Every time you read in a character using fgetc(inFile, &temp) that is a space, you increment the counter.

score 0 · Answer 3 · answered Nov 24 '17 at 05:44

0

In your current code you simply want to count the words. Therefore you are not interested in the words themselves. You can suppress the assignment with the optional * character:

    fscanf(inFile, "%*s");

answered Nov 24 '17 at 05:44

Amin Negm-Awad

16,582
3
35
50

Stack Smashing and using malloc

3 Answers3