Thread: Segmentation fault

  1. #1
    Registered User
    Join Date
    Dec 2009
    Posts
    24

    Segmentation fault

    I'm creating a program that reads text files, each with a priority and message, and then prints them in the correct order. Files with higher priority are printed first, and if they have the same priority, the ones that were entered in first are printed first. My code compiles fine, but if more than 3 text files are entered, I get a Segmentation fault. It also has a problem sorting by priority. Is there a way to fix this without significantly changing my code?

    Code:
    #include <iostream>
    #include <fstream>
    #include <string>
    #include <cassert>
    using namespace std;
    
    
    /* ----------------------- Package Class ----------------------- */
    class package
    { 
    	public:
    	int priority;
    	int order;
    	string message;
    };
    
    
    /* ----------------------- Main Program ----------------------- */
    int main()
    {
    
    /* ----------------------- The Variables ----------------------- */
    	int amount;
    	package temp;
    	package *packages;
    	string fileName;
    	ifstream fin;
    	 
    /* ----------------------- Opening the Files ----------------------- */
    	cout << "\nEnter the amount of packages you want entered: " << endl;
    	cin >> amount;
    	
    	packages = new package[amount];
    	if (packages == 0) 
    		cout << "Memory Cannot Be Allocated";
    	
    	cout << "\nEnter the names of the packages one by one:" << endl;
    
    	for (int i=0; i<amount; i++)
    	{
    
    		cout << "Package " << i+1 << ": ";
    		cin >> fileName;
    		fin.open(fileName.data(), ios::in);
    		assert( fin.is_open() );
    		
    		fin >> packages[i].priority;	
    		getline(fin, packages[i].message);
    		packages[i].order = i+1;
    		
    		fin.close();
    	}
    
    
    /* ----------------------- Sorting the Packages ----------------------- */
    	for (int i=1; i<amount-1; i++)
    	{
    		for (int j=0; j=amount-i; j++)
    		{
    			if (packages[j].priority > packages[j+1].priority)
    			{
    				packages[j] = temp;
    				packages[j] = packages[j+1];
    				packages[j+1] = temp;
    			}
    			
    			if (packages[j].priority == packages[j+1].priority)
    			{
    				if (packages[j].order > packages[j+1].order)
    				{
    					packages[j] = temp;
    					packages[j] = packages[j+1];
    					packages[j+1] = temp;
    				}
    			}
    		}
    	}
    
    /* ----------------------- Printing the Files ----------------------- */
    
    
    	cout << "The Packages:";
    	
    	for (int i=0; i<amount; i++)
    	{
    		cout << "\n\n" << "Priority: "<< packages[i].priority << "\nOrder: " << packages[i].order << "\n" << packages[i].message;
    	}
    		
    	cout << endl << endl;
    
    	delete[] packages;
    
    
    return 0;
    }
    Here is an example text file: (the first character is the priority, 1 being highest)
    Code:
    1This program is awesome
    Thanks so much for the help!
    Last edited by M-S-H; 05-05-2010 at 07:18 PM.

  2. #2
    and the Hat of Guessing tabstop's Avatar
    Join Date
    Nov 2007
    Posts
    14,336
    Code:
    packages[j] = temp;
    packages[j] = packages[j+1];
    packages[j+1] = temp;
    Not a very good example of swapping. (I.e., you kill a packages.)

  3. #3
    Registered User
    Join Date
    Dec 2009
    Posts
    24
    yeah, its basically bubble sort...
    so what would be a better way?

  4. #4
    and the Hat of Guessing tabstop's Avatar
    Join Date
    Nov 2007
    Posts
    14,336
    I know you're trying to do bubblesort. I know that you intend those three lines to swap two variables. The problem is, they don't. Trace it out and see.

  5. #5
    Registered User
    Join Date
    Dec 2009
    Posts
    24
    well i thought this change would make it work, but it didn't
    Code:
    temp = packages[j];
    packages[j] = packages[j+1];
    packages[j+1] = temp;

  6. #6
    and the Hat of Guessing tabstop's Avatar
    Join Date
    Nov 2007
    Posts
    14,336
    Did you change both of them? EDIT: Also, this looks suspicious:
    Code:
    for (int j=0; j=amount-i; j++)
    Last edited by tabstop; 05-05-2010 at 07:45 PM.

  7. #7
    Registered User
    Join Date
    Dec 2009
    Posts
    24
    i did change both to make temp = packages[j];
    and j=amount-i should have been j<amount-i, but still no luck

    Here's the updated code:
    Code:
    /************* 
    Michael Hickman
    ECE 231
    *************/
    
    #include <iostream>
    #include <fstream>
    #include <string>
    #include <cassert>
    using namespace std;
    
    
    /* ----------------------- Package Class ----------------------- */
    class package
    { 
    	public:
    	int priority;
    	int order;
    	string message;
    };
    
    
    /* ----------------------- Main Program ----------------------- */
    int main()
    {
    
    /* ----------------------- The Variables ----------------------- */
    	int amount;
    	package temp;
    	package *packages;
    	string fileName;
    	ifstream fin;
    	 
    /* ----------------------- Opening the Files ----------------------- */
    	cout << "\nEnter the amount of packages you want entered: " << endl;
    	cin >> amount;
    	
    	packages = new package[amount];
    	if (packages == 0) 
    		cout << "Memory Cannot Be Allocated";
    	
    	cout << "\nEnter the names of the packages one by one:" << endl;
    
    	for (int i=0; i<amount; i++)
    	{
    
    		cout << "Package " << i+1 << ": ";
    		cin >> fileName;
    		fin.open(fileName.data(), ios::in);
    		assert( fin.is_open() );
    		
    		fin >> packages[i].priority;	
    		getline(fin, packages[i].message);
    		packages[i].order = i+1;
    		
    		fin.close();
    	}
    
    
    /* ----------------------- Sorting the Packages ----------------------- */
    	for (int i=1; i<amount-1; i++)
    	{
    		for (int j=0; j<amount-i; j++)
    		{
    			if (packages[j].priority > packages[j+1].priority)
    			{
    				temp = packages[j];
    				packages[j] = packages[j+1];
    				packages[j+1] = temp;
    			}
    			
    			if (packages[j].priority == packages[j+1].priority)
    			{
    				if (packages[j].order > packages[j+1].order)
    				{
    					temp = packages[j];
    					packages[j] = packages[j+1];
    					packages[j+1] = temp;
    				}
    			}
    		}
    	}
    
    /* ----------------------- Printing the Files ----------------------- */
    
    
    	cout << "The Packages:";
    	
    	for (int i=0; i<amount; i++)
    	{
    		cout << "\n\n" << "Priority: "<< packages[i].priority << "\nOrder: " << packages[i].order << "\n" << packages[i].message;
    	}
    		
    	cout << endl << endl;
    
    	delete[] packages;
    
    
    return 0;
    }

  8. #8
    and the Hat of Guessing tabstop's Avatar
    Join Date
    Nov 2007
    Posts
    14,336
    I can run it fine in both Windows and Linux. Are you getting a segfault or an assertion failure? Can you run a debugger on it?

  9. #9
    Registered User
    Join Date
    Dec 2009
    Posts
    24
    I no longer get a segmentation fault, but its not sorting them correctly. Once a priority 3 was printed before a priority 1, then a 2 was first, then 1, then 3; then with everything was sorted correctly....

  10. #10
    and the Hat of Guessing tabstop's Avatar
    Join Date
    Nov 2007
    Posts
    14,336
    Compare what you've got with what you've posted, and make sure you recompiled -- the above is giving correct results.

  11. #11
    Registered User
    Join Date
    Dec 2009
    Posts
    24
    yeah, i did, and its still not sorting right.
    For example, the two text files:

    First:
    Code:
    3I hope this works
    Second:
    Code:
    1This program is awesome
    The second text file entered should come out first, but it doesn't.

  12. #12
    and the Hat of Guessing tabstop's Avatar
    Join Date
    Nov 2007
    Posts
    14,336
    If you only have two elements in your list, your bubble sort doesn't run (the i loop is immediately false).

  13. #13
    Registered User
    Join Date
    Dec 2009
    Posts
    24
    How so? the first message has a priority of 3 and the second 1, so wouldn't is swap them?
    Oh, got it.... so i could just create an if statement to correct this problem? if amount == 2, then another if statement to swap them if the first has less priority than the other?

  14. #14
    and the Hat of Guessing tabstop's Avatar
    Join Date
    Nov 2007
    Posts
    14,336
    Your bubble sort doesn't run. i begins at 1, amount is 2, and therefore the for loop test fails immediately. (You should have <=, not <, which we managed to miss for all this time.)

  15. #15
    Registered User
    Join Date
    Dec 2009
    Posts
    24
    cool, never mind my edit then.... let me try that.

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Segmentation fault problem
    By odedbobi in forum Linux Programming
    Replies: 1
    Last Post: 11-19-2008, 03:36 AM
  2. Segmentation fault
    By bennyandthejets in forum C++ Programming
    Replies: 7
    Last Post: 09-07-2005, 05:04 PM
  3. Segmentation fault
    By NoUse in forum C Programming
    Replies: 4
    Last Post: 03-26-2005, 03:29 PM
  4. Locating A Segmentation Fault
    By Stack Overflow in forum C Programming
    Replies: 12
    Last Post: 12-14-2004, 01:33 PM
  5. Segmentation fault...
    By alvifarooq in forum C++ Programming
    Replies: 14
    Last Post: 09-26-2004, 12:53 PM