Solved

Why GPF on code?

Posted on 1997-11-14
8
206 Views
Last Modified: 2010-04-10
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;
}
0
Comment
Question by:whatever080697
[X]
Welcome to Experts Exchange

Add your voice to the tech community where 5M+ people just like you are talking about what matters.

  • Help others & share knowledge
  • Earn cash & points
  • Learn & ask questions
8 Comments
 
LVL 22

Accepted Solution

by:
nietod earned 200 total points
ID: 1173179
I'll answer in a second.
0
 

Expert Comment

by:mwalsh111097
ID: 1173180
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.  
0
 
LVL 22

Expert Comment

by:nietod
ID: 1173181
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.)
0
Industry Leaders: We Want Your Opinion!

We value your feedback.

Take our survey and automatically be enter to win anyone of the following:
Yeti Cooler, Amazon eGift Card, and Movie eGift Card!

 
LVL 22

Expert Comment

by:nietod
ID: 1173182
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...

0
 
LVL 2

Expert Comment

by:wpinto
ID: 1173183
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
0
 
LVL 2

Expert Comment

by:wpinto
ID: 1173184
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.  

0
 
LVL 22

Expert Comment

by:nietod
ID: 1173185
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.
0
 

Author Comment

by:whatever080697
ID: 1173186
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.
0

Featured Post

Free Tool: IP Lookup

Get more info about an IP address or domain name, such as organization, abuse contacts and geolocation.

One of a set of tools we are providing to everyone as a way of saying thank you for being a part of the community.

Question has a verified solution.

If you are experiencing a similar issue, please ask a related question

Suggested Solutions

Container Orchestration platforms empower organizations to scale their apps at an exceptional rate. This is the reason numerous innovation-driven companies are moving apps to an appropriated datacenter wide platform that empowers them to scale at a …
Basic understanding on "OO- Object Orientation" is needed for designing a logical solution to solve a problem. Basic OOAD is a prerequisite for a coder to ensure that they follow the basic design of OO. This would help developers to understand the b…
The goal of the tutorial is to teach the user how to use functions in C++. The video will cover how to define functions, how to call functions and how to create functions prototypes. Microsoft Visual C++ 2010 Express will be used as a text editor an…
The viewer will learn additional member functions of the vector class. Specifically, the capacity and swap member functions will be introduced.

763 members asked questions and received personalized solutions in the past 7 days.

Join the community of 500,000 technology professionals and ask your questions.

Join & Ask a Question