Link to home
Start Free TrialLog in
Avatar of whatever080697
whatever080697

asked on

Why GPF on code?

Why oh why, does this code give me a general protection fault with a txt file of 30 lines and not one of 4 lines?
And also, how do I correct the code to make use of the 30 line txt file?
//create 30 instances of strings.
#include<iostream.h>
#include<fstream.h>
#include<stddef.h>

int main()
{
ifstream Dictionary;
Dictionary.open("swords.txt");
if (!Dictionary)
 cout<<"can't open dictionary!"<<endl;
char *ptrWord[33];
//**********************************************************
char dummy;
for (int i = 0;i<33;i++)
 {
  ptrWord[i] = new char[70]; //create 32 instances of 68 //char + \0 char arrays.
 if (ptrWord[i] == NULL)
  return 1;
 }; i = 0;
 Dictionary.get(ptrWord[i],65);
while (Dictionary)
 {
  i++;

      Dictionary.get(ptrWord[i],65);
      Dictionary.get(dummy);
        cout<<ptrWord[i]<<endl;
 };
  delete [] ptrWord;
  return 0;
}
ASKER CERTIFIED SOLUTION
Avatar of nietod
nietod

Link to home
membership
This solution is only available to members.
To access this solution, you must be a member of Experts Exchange.
Start Free Trial
Avatar of mwalsh111097
mwalsh111097

Sorry about this two-aprt answer, but when I type long answers I usually find that someone else has answered it while I was typing.

I see a bunch of problems.  Its hard to guess what is causing the  general protection fault.  I'll ofter some suggestions and I that doesn't fix it, we'll try again.

1. This probably isn't the cause of the GP fault, but you declare the PtrWord array to have 33 entries (0-32).  Your "for" loop initializes all 33 of the entries (0-32) but the comment says it initinitializes 32.  If you are using the last pointer as a terminator, that would be a problem, but I don't believe you are.

2.  This would cause a GP fault or other error, but should not depend on the file size.  You delete the PtrWord array, but that array is not dynamically allocated, that is, you did not write

char * ptrWord = new[33];

Since you did not allocate the array you must not delete.  However, you should delete each of dynamically allocated string whose pointers are stored in the array.  i.e.

for (i = 0; i < 33; ++i)
   delete ptrWord[i];

3.  I think that your logic for gettng the lines from the file is a little weird, possibly buggy, although you might be doing something intentional that I'm missing.  You get the first line outside of the loop.  The rest of the lines are obtained inside the loop.  Inside the loop you get and discard the newline, but you don't for the first line outside of the loop.  That measn the first line read inside the loop will be empty (the end of the first line.  You might want to try something like

do
{
   Dictionary.Get(ptrWord[i],65);
   Dictionary.Get(Dummy);
   ++i;
} while(Dictionary);

I suspect this can be improved on, but I'm not too familiar with streams and the available functions.

If this doesn't fix the problem, post the new code.  One thing you might want to test is that the loop that reads the data from the file is not going past the end of the array.  
Although the delete is a problem, the space before the [] is not.  You are allowed to have a space (any white space) between the delete and the [].

The incrementing does not seem to be a problem.  Its just that the design is weird in that the first line is read before the loop.  However, I suspect that the problem is closely related.  I suspect that you are going past the end of the array.  Why I don't know.  If the file has too many lines it might.  Or if a line is two long and gets read as two lines.  Or if it is because the first line takes up two array entries (because you don't remove the newline after reading the first line.)
Hi whatever,

Replace your code with the foll code & it should work fine. The problem is definitely with the delete operator but not in the way that mwalsh described. The point is to delete all elements that have been alloacted as shown below in my code

Also note that getline() is the proper way to get the strings (in this example). The dummy variable is now not required.

int main()
{
ifstream Dictionary;
Dictionary.open("swords.txt");
if (!Dictionary) {
      cout<<"can't open dictionary!"<<endl;
      return 1;
}

char *ptrWord[33];
//**********************************************************
for (int i = 0;i<33;i++)
{
      ptrWord[i] = new char[70]; //create 32 instances of 68 //char + \0 char arrays.
      if (ptrWord[i] == NULL)
            return 1;
};

i = 0;
while (Dictionary)
{
      Dictionary.getline(ptrWord[i],65);
      cout<<ptrWord[i]<<endl;

      i++;
};

for (i=0; i<33; i++)
      delete [] ptrWord[i];

return 0;
}

Hope this helps,

Wilfred

Note: I tried posting this as an answer awhile back, but I lost this entire text since I got the message that it has already been answered. I had to retype this entire text all over again. Nietod, I seriously thought that you would answer in a second, looks like lots of seconds have passed by...

Hi whatever,

Please note that the correct way to delete is
for (i = 0; i < 33; ++i)
   delete [] ptrWord[i];

& not
for (i = 0; i < 33; ++i)
   delete ptrWord[i];

even though the latter may work at the present, it will definitely fail at some time (see the documentation on the delete operator)

Also note that the code which I posted earlier will work fine for all cases except
1. If there are more than 33 lines of text in the file
2. If any line contains more than 65 chars

The above code was not designed to be robust, but to solve your problem

Wilfred
wpinto is correct that the correct way to clean up the array is

for (i=0; i<33; +=i)
   delete [] ptrWord[i];

I had forgotten the [].  (Actually, though its unlikely it would ever be a problem, but its best to not take chances.)

Also wpinto's suggestings for dealing with streams are good ideas, as I said, I wasn't familiar with streams.

The change to use

Dictionary.getline(ptrWord[i],65);

instead of the two get()'s is good and likely to fix you current bug.  (I though there was a way to do that with one function call, I'm just not familiar with streams.)  The reason I think it will fix things is that I think that you are using the first two entries in your array for the first line because you don't get the newline after reading the first line.  This should fix that cleanly.  

Thank you all for the tips.  Unfortunately my problem is the text file that I used.  The new line character wasn't input correctly somewhere within the file, so that the input line read into memory causing a fault.  I carefully created a text file,and made sure that I hit the enter key after every line.  With the new txt file my code above worked. Also, I thought that before entering a while loop, whose condition is EOF and, when the programmer wants to read from said file, it is a good idea to read once before entering?  I can't quote that, but maybe I misread it.  Once again, I am sincerely thankfull for the response I have recieved.
Avatar of whatever080697

ASKER

The text file may have been A problem, but it was not THE problem.  You will want to consider the points we made, especialy if your program needs to be more robust and not crash when given a bad input file.  (I think that happens more times than not.)  For example, it is a bad idea to assume that the file has lines of a particular length or a particular number of lines.  Either of these problems could cause your program to crash, but could be checked for without too much difficulty.