GAAAH! Memory issue?

[FireSBurnsmuP]

Alright, my program is giving me some serious problems. I think I have a memory leak, but I shouldn't have any.

I have ifstream In; , followed by In.open(thefile.c_str()); , a series of In.get( tempChar ); s, then at the end of the function I have In.close(); .

The problem was that it was getting to line 24, outputting something, then never outputting line 25. It froze. My syntax is entirely correct there, but I think I have a memory leak causing it to freeze. What's my problem?

/* this is c++. I am using g++ on a debian 'etch' linux machine. */

[John A]

Look for pointers and dynamic memory allocation. Oh and if you want us to help you find the error, maybe you should post some code (or at least up to the point where it freezes). Thanks.

[FireSBurnsmuP]

please god, thanks. Here's the driver:

#include <iostream>
#include <vector>
#include <fstream>
#include <string>
#include <iomanip>
using namespace std;

#include "proj11mineSweeperGame.h"

int main()
{
    mineSweeperGame Game;
    string sMap;
    cout << "Please type in the name of the map file you wish to use:\n";
    cin >> sMap;
    Game.readMap(sMap);
    
    system ("pause");
    return 0;
}

Now here's the implementation file:

#include <iostream>
#include <fstream>
#include <iomanip>
#include <vector>
#include <string>
#include <cstdlib>
using namespace std;

#include "proj11mineSweeperGame.h"

void mineSweeperGame::readMap( string sMap ) // reads the map file and sets up the maps
{
    ifstream In;
    
    In.open( sMap.c_str() );
    while ( In == NULL )
    {
        cout << "open failure. Please check the file's location and try again.\n";
        cout << "What is the map file's name? ";
        cin >> sMap;
        
        In.open( sMap.c_str() );
    }
    cout << "24\n" << "25\n";
    char tempChar;
    In.get( tempChar );
    unsigned int i = 0;
    while ( tempChar != '/n' ) // loop to find the row and col numbers.
    {
        string tempString;
        
        if ( isdigit( tempChar ) )
        {
            if ( i > 2 )
            {
                tempString = tempChar;
                row = atoi( tempString.c_str() );
            }
            
            else
            {
                tempString = tempChar;
                col = atoi( tempString.c_str() );
            }
        }
        
        In.get(tempChar);
        i++;
    }
    cout << "50\n";
    // time to set up the map vectors.
    unsigned int x;
    // i is already declared
    for ( i = 0 ; i < col ; i++ ) // the loop to set up the columns.
    {
        vector <char> emptyCol(row, '#');// set up cols with the rows filled with #s
        map.push_back(emptyCol); // put them into the map they can see.
        
        vector <char> emptyCol2(row, ' ');// set up cols with the rows filled with spaces
        txtMap.push_back(emptyCol2); // put them into the map they don't see.
    }
    cout << "62\n";
    // nested loop to find and place the mines in txtMap
    for ( i = 0 ; i < col ; i++ ) // tells what column I'm in, and repeats only that many times
    {
        x = 0;
        In.get( tempChar );
        while (  tempChar != '\n'  ) // go until a newline
        {
            if ( isdigit( tempChar ) ) // if it's a number, process it.
            {
                if ( tempChar == 1 ) // if it's a one, there's a mine
                {
                    txtMap[i][x] = 'X'; // X marks the spot XD
                }
                // if it's a 0, I want a ' ' there; empty.
                In.get( tempChar );
                x++;
            }
            
            else /* if it isn't a number, it needs to be skipped. 
            why'd you have to put spaces in? haha, kidding.
            I would've put support in anyways. XD */
            {
                In.get( tempChar );
            }
        }
    }    
    // the map they see doesn't have the mines in it. i have the isMine function to tell that
    cout << "90\n";
    cout << "\n Map processed.\n"; // thought they should know that it was done successfully
    In.close();
    return;
};

ostream& operator<< (ostream& Out, mineSweeperGame sMap)
{
    cout << "\n  ";
    unsigned int i;
    for ( i = 0 ; i < sMap.col ; i++) // output the indicatory column numbers
    {
        cout << i << "  ";
    }
    cout << "\n";
    for ( i = 0 ; i < sMap.row ; i++ ) // output the rows with indicatory row numbers
    {
        cout << i << "|";
        for (unsigned int x = 0 ; x < sMap.col ; x++ )
        {
            cout << sMap.map[x][i] << "|";
        }
        cout << "\n";
    }
    cout << "==========================================\n";
};

And finally the header:

#include <iostream>
#include <fstream>
#include <vector>
using namespace std;

class mineSweeperGame
{
    public:
        void readMap( string sMap );
        friend ostream& operator<< ( ostream& Out, mineSweeperGame game );
        bool isMine( int usrRow, int usrCol );
        
    private:
        vector< vector<char> > txtMap; //the map as is in text file
        vector< vector<char> > map; // the map as seen by the player
        int row, // the number of rows in the map
            col; // the number of columns in the map
};

Thanks a lot. The faster this gets done, the better, I only have 10 minutes to finish this. CRAP!

[~s.o.s~]

Hmm...here is what I think you should do:

1. Always keep a guard condition while making header files which does away with the problem of recursive inclusion of headers. Something like:

#ifndef TGA_H_
#define TGA_H_

// your header file here

#endif

2. Don't know why but your loop looks fishy:

while ( In == NULL )
    {
        cout << "open failure. Please check the file's location and try again.\n";
        cout << "What is the map file's name? ";
        cin >> sMap;
        
        In.open( sMap.c_str() );
    }

Try removing the loop and ducking out if the fle is not found.

3. Tada..and this I think according to me is the culprit:

while ( tempChar != '/n' ) // loop to find the row and col numbers.

Just to let you know there is no such thing as /n, it must have been a typo mistake since you should write \n if validating against a newline character.

4. Oh yes...don't use system("pause") , getchar( ) achieves the same thing without putting portability at stake....

[Ancient Dragon]

if ( i > 2 )
            {
                tempString = tempChar;
                row = atoi( tempString.c_str() );
            }
            
            else
            {
                tempString = tempChar;
                col = atoi( tempString.c_str() );
            }

The above is much too complicated. All it is doing is converting a single character to an integer. Just simply subtract '0'.

if ( i > 2 )
            {
                row = tempChar - '0';
            }
            
            else
            {
                col = tempChar - '0';
            }

[FireSBurnsmuP]

oh... yep. That's a pretty typical typo with me.

The while ( In == NULL ) loop actually works perfectly. That's how I've always done that.

I never did understand the stuff to prevent the recursive header thing, I don't really get it yet. I'll have to ask someone to explain that for me.

And why would subtracting '0' from a character convert it to an integer? isn't that what atoi is for? granted, atoi is meant to be used with c-strings but it still gets the job done for characters. I don't remember if it worked without the addition of the string converted to a c-string though... that part gave me a lot of hell.

and oh yeah... system("pause") is a dos thing... that wouldn't have worked if my program ever got that far ^_^U

Alrighty, well thanks a lot guys. I didn't get this turned in on time, but I'm still going to finish writing it.

[Ancient Dragon]

The while ( In == NULL ) loop actually works perfectly. That's how I've always done that.

I see your logic -- if the file was not found, then ask for a new file name.

I never did understand the stuff to prevent the recursive header thing, I don't really get it yet. I'll have to ask someone to explain that for me.

Its for instances when a .h file is included two or more times, which often happens with your compiler's standard header files. This prevents symbols from being defined more than once.

And why would subtracting '0' from a character convert it to an integer?

lets take the example of '1'. '1' - '0' = 49 - 48 = 1. You can use a good ascii chart to see all the decimal values of the characters. I like to use '0' in my programs instead of 48 because it makes more sense.

[FireSBurnsmuP]

wow, thanks. Now I get it, it's the decimal values.

while I'm at it, for the #ifndef stuff, should I be putting the class name in there instead of a TGA_H? I thought I had seen it done that way before...

[Infarction]

while I'm at it, for the #ifndef stuff, should I be putting the class name in there instead of a TGA_H? I thought I had seen it done that way before...

Leave it as TGA_H. The #ifndef followed by the #define are a way to tag your file uniquely so that it never gets #included more than once (which would cause multiple-definition hell). If you use an IDE which autogenerates those lines (old versions of VC++ used to, but I think newer ones use the compiler-specific #pragma once ) you'd see pretty much a random string so there would never be issues with another file having the same tag as yours.

[Ancient Dragon]

while I'm at it, for the #ifndef stuff, should I be putting the class name in there instead of a TGA_H? I thought I had seen it done that way before...

I always use the header filename, but it doesn'r really matter as long as it isn't duplicated anywhere else.

[FireSBurnsmuP]

Oh, ok. So it really doesn't matter what that says as long as it isn't also defined somewhere else. Well thanks, guys.