Trying to create a program to read a users input then break the array into seperate words are my pointers all valid?

Question

char **findwords(char *str);
int main()
{
int test;
char words[100]; //an array of chars to hold the string given by the user
char **word;  //pointer to a list of words
int index = 0; //index of the current word we are printing
char c;
cout << "die monster !";
//a loop to place the charecters that the user put in into the array  

do {
    c = getchar();
    words[index] = c;
} while (words[index] != '\n');



word = findwords(words);

while (word[index] != 0) //loop through the list of words until the end of the list
{

    printf("%s\n", word[index]); // while the words are going through the list print them out
    index ++; //move on to the next word
}

    //free it from the list since it was dynamically allocated
free(word);
cin >> test;

return 0;
}

char **findwords(char *str)
{

int size = 20; //original size of the list 
char *newword; //pointer to the new word from strok
int index = 0; //our current location in words
char **words = (char **)malloc(sizeof(char *) * (size +1)); //this is the actual list of words


/* Get the initial word, and pass in the original string we want strtok() *
*   to work on. Here, we are seperating words based on spaces, commas,   *
 *   periods, and dashes. IE, if they are found, a new word is created.   */

newword = strtok(str, " ,.-");

while (newword != 0) //create a loop that goes through the string until it gets to the end
{
    if (index == size)
    {
        //if the string is larger than the array increase the maximum size of the array
        size += 10;
        //resize the array
        char **words = (char **)malloc(sizeof(char *) * (size +1));
    }
    //asign words to its proper value
    words[index] = newword;
    //get the next word in the string
    newword = strtok(0, " ,.-");
    //increment the index to get to the next word
    ++index;

}
words[index] = 0;



return words;
}

break the array into the individual words then print them out th

Answer 1

I think there is a bug memory leakage because you first allocate

    char **words = (char **)malloc(sizeof(char *) * (size +1)); //when declaring

when declaring the variable, and after that you again allocate the same **words in the loop body:

    char **words = (char **)malloc(sizeof(char *) * (size +1)); // in the while loop

The above line in the while loop with which you allocate the space to store the string should be (1)

    //in the while loop should be
    char *words[index] = (char *)malloc(sizeof(char ) * (size +1));
    strcpy (words[index], str);

Or simply (2)

    words[index] = str;

Because the str already points to a valid memory location which you assign to the array of pointers.

In the (1) method above you are allocating a block of memory of size+1 of type char and copying the string in str into words[index] with strcpy. For this you require to reserve a memory location into words[index] first and then perform the copy. If this is the case the the memory freeing is not at simple as free (word) instead each of the allocated block will need to be manually removed.

 for (index = 0; words[index] != 0; index++)
 {
   free (words[index];
 }
 free (words);

In the (2) solution is in my opinion not a good one, because you have passed a pointer to a string and assign that pointer value to store the string. So both the str and the words[index] point to the same location. Now after the function returns if anybody frees str (if it was dynamically allocated) then the words[index] reference will become illegal.

EDIT:

Also you need to use

gets (words); or in using c++ cin >> words; or use getline, or simply increment the index counter in your code, and assign a null at the end to terminate the string.

in main function. You do not increment the index counter so all the characters are assigned in the same location.

Answer 2

   do {
        c = getchar();
        words[index] = c;
    } while (words[index] != '\n');

you should also add '\0' at the end of your string (after the loop) in "words" array

You are not incrementing index this way you save only the last c

you should do while(word[index] != '\0') not while(word[index] != 0 ('\0' indicates end of line no 0)

  while (word[index] != 0) //loop through the list of words until the end of the list
    {

        printf("%s\n", word[index]); // while the words are going through the list print them out
        index ++; //move on to the next word
    }

Answer 3

I think everybody is trying to do it the hard way.

The std streams already break the input into words using the >> operator. We just need to be more careful on how we define a word. To do this you just need to define an ctype facet that defines space correctly (for the context) and then imbue the stream with it.

#include <locale>
#include <string>
#include <sstream>
#include <iostream>

// This is my facet that will treat the ,.- as space characters and thus ignore them.
class WordSplitterFacet: public std::ctype<char>
{
    public:
        typedef std::ctype<char>    base;
        typedef base::char_type     char_type;

        WordSplitterFacet(std::locale const& l)
            : base(table)
        {
            std::ctype<char> const&  defaultCType  = std::use_facet<std::ctype<char> >(l);

            // Copy the default value from the provided locale
            static  char data[256];
            for(int loop = 0;loop < 256;++loop) { data[loop] = loop;}
            defaultCType.is(data, data+256, table);

            // Modifications to default to include extra space types.
            table[',']  |= base::space;
            table['.']  |= base::space;
            table['-']  |= base::space;
        }
    private:
        base::mask  table[256];
};

Now the code looks very simple:

int main()
{
    // Create the facet.
    std::ctype<char>*   wordSplitter(new WordSplitterFacet(std::locale()));

    // Here I am using a string stream.
    // But any stream can be used. Note you must imbue a stream before it is used.
    // Otherwise the imbue() will silently fail.
    std::stringstream   teststr;
    teststr.imbue(std::locale(std::locale(), wordSplitter));

    // Now that it is imbued we can use it.
    // If this was a file stream then you could open it here.
    teststr << "This, stri,plop";

    // Now use the stream normally
    std::string word;
    while(teststr >> word)
    {
        std::cout << "W(" << word << ")\n";
    }
}

Testing:

> ./a.out
W(This)
W(stri)
W(plop)

With a correctly imbues stream we can use the old trick of copying from a stream into a vector:

std::copy(std::istream_iterator<std::string>(teststr),
          std::istream_iterator<std::string>(),
          std::back_inserter(data)
         );

Answer 4

Lots of issues:

1. In your first loop you are forgetting to increment index after each read character.

2. Also, if you have more than 100 characters, your program will likely crash.

3. getchar returns an "int". Not a char. Very important - especially if you input is redirected or piped in.

Try this instead:

  int tmp;
  tmp = getchar();
  while ((index < 99) && (tmp >= 0) && (tmp != '\n'))
  {
      word[index] = (char)tmp;
      tmp = getchar();
      index++;
  }
  word[index] = 0; /* make life easier - null terminate your string */

Your "findwords" function scares the hell out of me. You haven't don't have enough points on S.O. for me to elaborate on the issues here. In any case

Answer 5

I'm tempted to open with some lame crack about the '80s calling and wanting their obsolete "C++ as a better C" code back, but I'll try to restrain myself and just give at least some idea of how you might consider doing something like this:

std::string line;

// read a line of input from the user:
std::getline(line, std::cin);

// break it up into words:
std::istringstream buffer(line);    
std::vector<std::string> words((std::istream_iterator<std::string>(buffer)), 
                                std::istream_iterator<std::string>());

// print out the words, one per line:
std::copy(words.begin(), words.end(), 
          std::ostream_iterator(std::cout, "\n"));