Like Tree1Likes
  • 1 Post By Salem

Segmentation Fault issue

This is a discussion on Segmentation Fault issue within the C++ Programming forums, part of the General Programming Boards category; Hi, For my final project this semester, we were asked to make a modified poker-like game in which the player ...

  1. #1
    Registered User
    Join Date
    Feb 2012
    Posts
    16

    Segmentation Fault issue

    Hi,

    For my final project this semester, we were asked to make a modified poker-like game in which the player must try his/her best to match a type of poker hand and they are paid in accordance to what hand they create and how much they wager. Anyway, I believe I have most of the assignment finished (I still have to setup a wager system, a better card printing function, and a few other things). Anyway, The code I have this far is giving me a segmentation fault after completing one cycle of the while loop in the main function. I tried debugging my code with valgrind and it is directing me to my call to "cin." I cannot figure out why cin would cause a segfault. Anyway, here's my code. Different code blocks represent different source and header files

    Homework5.cpp
    Code:
    //============================================================================
    // Name        : Homework5.cpp
    // Author      : 
    // Version     :
    // Copyright   : Your copyright notice
    // Description : Hello World in C++, Ansi-style
    //============================================================================
    
    #include <iostream>
    #include "hand.h"
    #include "deck.h"
    #include "prototypes.h"
    using namespace std;
    
    int main()
    {
        hand myHand;
        deck myDeck;
        int i,j, handType;
        string answer;
        bool playAgain = true;
    
        //play game
        while(playAgain == true)
        {
            myDeck.shuffle();
            //print out deck for debugging purposes only
            for(i=0;i<52;++i) cout<<myDeck.getValue(i)<<"*"<<myDeck.getSuit(i)<<endl;
    
            j = deal(myDeck,myHand);
            cout<<myHand.getValue(0)<<"*"<<myHand.getSuit(0)<<"  "<<myHand.getValue(1)<<"*"<<myHand.getSuit(1)<<"  "<<myHand.getValue(2)<<"*"<<myHand.getSuit(2)<<"  "<<myHand.getValue(3)<<"*"<<myHand.getSuit(3)<<"  "<<myHand.getValue(4)<<"*"<<myHand.getSuit(4)<<endl;
    
            for(i=0;i<5;++i)
            {
                cout<<"Discard card number "<<i+1<<"?";
                cin>>answer;
                cout<<endl;
    
                if(answer == "yes")
                {
                    myHand.setValue(i,myDeck.getValue(j));
                    myHand.setSuit(i,myDeck.getSuit(j));
                    ++j;
                }
                cout<<myHand.getValue(0)<<"*"<<myHand.getSuit(0)<<"  "<<myHand.getValue(1)<<"*"<<myHand.getSuit(1)<<"  "<<myHand.getValue(2)<<"*"<<myHand.getSuit(2)<<"  "<<myHand.getValue(3)<<"*"<<myHand.getSuit(3)<<"  "<<myHand.getValue(4)<<"*"<<myHand.getSuit(4)<<endl;
            }
    
            handType = myHand.handType();
    
            cout<<"Hand Type: "<<handType<<(handType > 0 ? '\a':' ')<<endl;
            cout<<endl<<"Play Again? ";
            cin>>answer;
            if(answer != "yes") playAgain = false;
        }
        return 0;
    }
    functions.cpp
    Code:
    #include "hand.h"
    #include "deck.h"
    
    int deal(deck &myDeck, hand &myHand)
    {
        int i;
        for(i=0; i<5;++i)
        {
            myHand.setValue(i,myDeck.getValue(i));
            myHand.setSuit(i,myDeck.getSuit(i));
        }
        return i;
    }
    card.h
    Code:
    #ifndef CARD_H_
    #define CARD_H_
    
    class card
    {
        private:
            int value;
            int suit; // 1=clubs, 2=spades,3=hearts,4=diamonds
    
        public:
    
            card(){setValue(2); setSuit(1);}
            card(int v, int c) {setValue(v); setSuit(c);}
    
            void setValue(int v) {value = v;}
            void setSuit(int c) {suit = c;}
            int getValue(){return value;}
            int getSuit(){return suit;}
    };
    
    
    
    #endif
    deck.h
    Code:
    #ifndef DECK_H_
    #define DECK_H_
    
    #include "card.h"
    
    class deck
    {
        private:
        card cards[52];
    
        public:
    
        deck();
    
        void setValue(int i, int v) {cards[i].setValue(v);}
        void setSuit(int i, int c) {cards[i].setSuit(c);}
        int getValue(int i) {return cards[i].getValue();}
        int getSuit(int i) {return cards[i].getSuit();}
    
        void shuffle(void);
    
    };
    #endif
    deck.cpp
    Code:
    #include <ctime>
    #include <cstdlib>
    #include <unistd.h>
    #include<iostream>
    #include "deck.h"
    using namespace std;
    
    deck::deck()
    {
        int i=0,j,k;
    
        for(j = 1; j < 5; ++j)
        {
            for(k = 2; k<15 ;++k)
            {
                cards[i].setValue(k);
                cards[i].setSuit(j);
                ++i;
            }
        }
    }
    
    void deck::shuffle()
    {
        card temp;
        int i,j;
        cout<<"shuffling..."<<endl;
        for(i = 0; i < 52; ++i)
        {
            temp = cards[i];
            sleep(1);
            srand(time(NULL));
            j = rand()%51;
            cards[i] = cards[j];
            cards[j] = temp;
        }
    }
    hand.h
    Code:
    #ifndef HAND_H_
    #define HAND_H_
    
    #include "card.h"
    #include <iostream>
    class hand
    {
        private:
            card cards[5];
    
            int valueCounter[15]; // J=11, Q=12, K=13, A=14
            int suitCounter[5];  // 1=clubs, 2=spades,3=hearts,4=diamonds
        public:
            hand() {}
    
            void initializeCounters() {int i=0; while(i<15) {valueCounter[i] = 0, suitCounter[i] = 0; ++i;}}
            void countCards(void);
    
            void setValue(int i, int v) {cards[i].setValue(v);}
            void setSuit(int i, int c) {cards[i].setSuit(c);}
            int getValue(int i) {return cards[i].getValue();}
            int getSuit(int i) {return cards[i].getSuit();}
    
            int handType(void);
            //string printHand(void);
    };
    #endif
    hand.cpp
    Code:
    #include "card.h"
    #include "hand.h"
    #include <iostream>
    using namespace std;
    void hand::countCards(void)
    {
        initializeCounters();
        int cardVal = 2, suitVal=1, cardNum=0;
    
        while(cardNum<5)
        {
            while(cardVal<15)
            {
                if(cardVal == cards[cardNum].getValue()) {++valueCounter[cardVal]; cardVal=2; break;}
                ++cardVal;
            }
            while(suitVal<5)
            {
                if(suitVal == cards[cardNum].getSuit()) {++suitCounter[suitVal]; suitVal=1; break;}
                ++suitVal;
            }
            ++cardNum;
        }
    }
    
    int hand::handType()
    {
        enum hands {none,onePair, twoPair, threeOfAKind, straight, flush, fullHouse, fourOfAKind, straightFlush, royalFlush};
    
        countCards();
        int i,j, pair = 0, threeKind = 0;
        for(i=1; i<5; ++i)
        {
            if(suitCounter[i] == 5)
            {
                if(valueCounter[10] == 1 && valueCounter[11] == 1 && valueCounter[12] == 1 && valueCounter[13] == 1 && valueCounter[14] == 1) return (int)royalFlush;
                else
                {
                    for(j=2;j<15;++j)
                    {
                        if(valueCounter[j] == 1 && valueCounter[j+1] == 1 && valueCounter[j+2] == 1 && valueCounter[j+3] == 1 && valueCounter[j+4] == 1) return (int)straightFlush;
                    }
                    return (int)flush;
                }
            }
    
            for(i=2;i<15;++i)
            {
                if(valueCounter[i] == 4) return (int)fourOfAKind;
                if(valueCounter[i] == 3) ++threeKind;
                if(valueCounter[i] == 2) ++pair;
            }
    
            if(pair == 1 && threeKind == 1) return (int)fullHouse;
            else if(pair == 2) return (int)twoPair;
            else if(pair == 1) return (int)onePair;
        }
        return none;    //none of the above
    }
    Any help would be much appreciated. Thanks! Below is the valgrind output and program output.

    shuffling...
    5*3
    7*2
    10*1
    13*1
    12*2
    13*3
    8*1
    8*4
    4*1
    10*4
    6*1
    13*2
    14*3
    12*3
    5*2
    7*4
    14*1
    6*2
    2*2
    8*3
    7*1
    14*4
    4*4
    10*2
    11*3
    9*2
    11*2
    12*1
    9*1
    7*3
    11*1
    14*2
    3*2
    9*4
    4*2
    8*2
    5*1
    3*4
    10*3
    4*3
    2*4
    6*3
    11*4
    13*4
    5*4
    12*4
    2*3
    3*3
    3*1
    9*3
    6*4
    2*1
    5*3 7*2 10*1 13*1 12*2
    Discard card number 1?yes

    13*3 7*2 10*1 13*1 12*2
    Discard card number 2?yes

    13*3 8*1 10*1 13*1 12*2
    Discard card number 3?yes

    13*3 8*1 8*4 13*1 12*2
    Discard card number 4?no

    13*3 8*1 8*4 13*1 12*2
    Discard card number 5?no

    13*3 8*1 8*4 13*1 12*2
    Hand Type: 2

    Play Again? yes
    ==26986== Invalid read of size 8
    ==26986== at 0x36D5A6F680: std::basic_istream<char, std::char_traits<char> >& std:perator>><char, std::char_traits<char>, std::allocator<char> >(std::basic_istream<char, std::char_traits<char> >&, std::basic_string<char, std::char_traits<char>, std::allocator<char> >&) (in /usr/lib64/libstdc++.so.6.0.16)
    ==26986== by 0x4012B3: main (Homework5.cpp:53)
    ==26986== Address 0xffffffffffffffe8 is not stack'd, malloc'd or (recently) free'd
    ==26986==
    ==26986==
    ==26986== Process terminating with default action of signal 11 (SIGSEGV)
    ==26986== Access not within mapped region at address 0xFFFFFFFFFFFFFFE8
    ==26986== at 0x36D5A6F680: std::basic_istream<char, std::char_traits<char> >& std:perator>><char, std::char_traits<char>, std::allocator<char> >(std::basic_istream<char, std::char_traits<char> >&, std::basic_string<char, std::char_traits<char>, std::allocator<char> >&) (in /usr/lib64/libstdc++.so.6.0.16)
    ==26986== by 0x4012B3: main (Homework5.cpp:53)
    ==26986== If you believe this happened as a result of a stack
    ==26986== overflow in your program's main thread (unlikely but
    ==26986== possible), you can try to increase the size of the
    ==26986== main thread stack using the --main-stacksize= flag.
    ==26986== The main thread stack size used in this run was 8388608.
    ==26986==
    ==26986== FILE DESCRIPTORS: 3 open at exit.
    ==26986== Open file descriptor 2: /dev/pts/0
    ==26986== <inherited from parent>
    ==26986==
    ==26986== Open file descriptor 1: /dev/pts/0
    ==26986== <inherited from parent>
    ==26986==
    ==26986== Open file descriptor 0: /dev/pts/0
    ==26986== <inherited from parent>
    ==26986==
    ==26986==
    ==26986== HEAP SUMMARY:
    ==26986== in use at exit: 29 bytes in 1 blocks
    ==26986== total heap usage: 3 allocs, 2 frees, 82 bytes allocated
    ==26986==
    ==26986== 29 bytes in 1 blocks are definitely lost in loss record 1 of 1
    ==26986== at 0x4A06FC7: operator new(unsigned long) (vg_replace_malloc.c:261)
    ==26986== by 0x36D5AA2E48: std::string::_Rep::_S_create(unsigned long, unsigned long, std::allocator<char> const&) (in /usr/lib64/libstdc++.so.6.0.16)
    ==26986== by 0x36D5AA3BBA: std::string::_Rep::_M_clone(std::allocator<char> const&, unsigned long) (in /usr/lib64/libstdc++.so.6.0.16)
    ==26986== by 0x36D5AA3C9F: std::string::reserve(unsigned long) (in /usr/lib64/libstdc++.so.6.0.16)
    ==26986== by 0x36D5A6F820: std::basic_istream<char, std::char_traits<char> >& std:perator>><char, std::char_traits<char>, std::allocator<char> >(std::basic_istream<char, std::char_traits<char> >&, std::basic_string<char, std::char_traits<char>, std::allocator<char> >&) (in /usr/lib64/libstdc++.so.6.0.16)
    ==26986== by 0x400FB6: main (Homework5.cpp:37)
    ==26986==
    ==26986== LEAK SUMMARY:
    ==26986== definitely lost: 29 bytes in 1 blocks
    ==26986== indirectly lost: 0 bytes in 0 blocks
    ==26986== possibly lost: 0 bytes in 0 blocks
    ==26986== still reachable: 0 bytes in 0 blocks
    ==26986== suppressed: 0 bytes in 0 blocks
    ==26986==
    ==26986== For counts of detected and suppressed errors, rerun with: -v
    ==26986== ERROR SUMMARY: 2 errors from 2 contexts (suppressed: 2 from 2)
    Segmentation fault (core dumped)
    Last edited by audifanatic518; 04-28-2012 at 12:28 PM.

  2. #2
    -bleh-
    Join Date
    Aug 2010
    Location
    somewhere in this universe
    Posts
    460
    trying to compile yours program. where is "prototypes.h"?
    "All that we see or seem
    Is but a dream within a dream." - Poe

  3. #3
    Registered User
    Join Date
    Feb 2012
    Posts
    16
    Quote Originally Posted by nimitzhunter View Post
    trying to compile yours program. where is "prototypes.h"?
    oops, sorry, I missed one. Here it is:

    prototypes.h
    Code:
    #ifndef PROTOTYPES_H_
    #define PROTOTYPES_H_
    
    int deal(deck &myDeck, hand &myHand);
    
    #endif /* PROTOTYPES_H_ */

  4. #4
    and the hat of int overfl Salem's Avatar
    Join Date
    Aug 2001
    Location
    The edge of the known universe
    Posts
    32,821
    Code:
    while(i<15) {valueCounter[i] = 0, suitCounter[i] = 0; ++i;}
    How many suits do you have?

    TBH, you need some constants in your code, such as
    Code:
    const int maxRank = 13;
    const int maxSuit = 4;
    const int deckSize = maxRank * maxSuit;
    const int handSize = 5;
    Use these to set your array sizes, and control your loop limits.
    Though having said that, have you considered replacing your arrays with std::vector ?

    Code:
            sleep(1);
            srand(time(NULL));
            j = rand()%51;
    If you just called srand() once at the start of main, then you wouldn't need to sleep (and make your program unnecessarily slow) just to get different results from rand()
    treenef likes this.
    If you dance barefoot on the broken glass of undefined behaviour, you've got to expect the occasional cut.
    If at first you don't succeed, try writing your phone number on the exam paper.
    I support http://www.ukip.org/ as the first necessary step to a free Europe.

  5. #5
    - - - - - - - - oogabooga's Avatar
    Join Date
    Jan 2008
    Posts
    2,808
    Code:
    void deck::shuffle()
    {
        card temp;
        int i,j;
        cout<<"shuffling..."<<endl;
        for(i = 0; i < 52; ++i)
        {
            temp = cards[i];
            sleep(1);
            srand(time(NULL));
            j = rand()%51;
            cards[i] = cards[j];
            cards[j] = temp;
        }
    }
    This is very very wrong. First the idea that you would have a sleep(1) in the loop ... bizarre. You're forcing it to take 52 seconds to complete. Get rid of the sleep.

    I can only assume that you put it there after realizing that without it, time(NULL) will return the same value 52 times in a row (since the loop will complete in well under a second). But the sleep is not the right solution.

    The proper thing to do is to simply move the srand(time(NULL)) out of the loop. Put it in deck::deck(). In general, it should only be executed once in the entire run of the program. (So normally you'd also have a static bool to determine if it's already been done so that if another deck was created it won't be done again, but you don't need to do that here.)

    Also, your shuffle algorithm is wrong. It should be
    Code:
    for (int i = 51; i > 0; i--) {
        int j = rand() % (i + 1);
        swap(card[i], card[j]);
    }
    The cost of software maintenance increases with the square of the programmer's creativity. - Robert D. Bliss

  6. #6
    Registered User
    Join Date
    Feb 2012
    Posts
    16
    Quote Originally Posted by Salem View Post
    Code:
    while(i<15) {valueCounter[i] = 0, suitCounter[i] = 0; ++i;}
    How many suits do you have?

    TBH, you need some constants in your code, such as
    Code:
    const int maxRank = 13;
    const int maxSuit = 4;
    const int deckSize = maxRank * maxSuit;
    const int handSize = 5;
    Use these to set your array sizes, and control your loop limits.
    Though having said that, have you considered replacing your arrays with std::vector ?

    Code:
            sleep(1);
            srand(time(NULL));
            j = rand()%51;
    If you just called srand() once at the start of main, then you wouldn't need to sleep (and make your program unnecessarily slow) just to get different results from rand()
    Quote Originally Posted by oogabooga View Post
    Code:
    void deck::shuffle()
    {
        card temp;
        int i,j;
        cout<<"shuffling..."<<endl;
        for(i = 0; i < 52; ++i)
        {
            temp = cards[i];
            sleep(1);
            srand(time(NULL));
            j = rand()%51;
            cards[i] = cards[j];
            cards[j] = temp;
        }
    }
    This is very very wrong. First the idea that you would have a sleep(1) in the loop ... bizarre. You're forcing it to take 52 seconds to complete. Get rid of the sleep.

    I can only assume that you put it there after realizing that without it, time(NULL) will return the same value 52 times in a row (since the loop will complete in well under a second). But the sleep is not the right solution.

    The proper thing to do is to simply move the srand(time(NULL)) out of the loop. Put it in deck::deck(). In general, it should only be executed once in the entire run of the program. (So normally you'd also have a static bool to determine if it's already been done so that if another deck was created it won't be done again, but you don't need to do that here.)

    Also, your shuffle algorithm is wrong. It should be
    Code:
    for (int i = 51; i > 0; i--) {
        int j = rand() % (i + 1);
        swap(card[i], card[j]);
    }
    Thanks to both of you for helping me out with the srand() thing. I knew there had to be a better solution. And yes, I used sleep() because otherwise I would get the same result from rand().

    Although, Huddha, the rest of my shuffle alorithm seems to be working fine.

  7. #7
    Registered User
    Join Date
    Feb 2012
    Posts
    16
    Quote Originally Posted by Salem View Post
    Code:
    while(i<15) {valueCounter[i] = 0, suitCounter[i] = 0; ++i;}
    How many suits do you have?

    TBH, you need some constants in your code, such as
    Code:
    const int maxRank = 13;
    const int maxSuit = 4;
    const int deckSize = maxRank * maxSuit;
    const int handSize = 5;
    Use these to set your array sizes, and control your loop limits.
    Though having said that, have you considered replacing your arrays with std::vector ?
    And thanks Salem, that solved the segFault issue. I was going out of bounds with that while loop, issue solved!

    As far as vectors are concerned, I have seen other people do that with poker games. unfortunately, we have not yet covered them in class, so i don't know how to use them.

  8. #8
    Internet Superhero
    Join Date
    Sep 2006
    Location
    Denmark
    Posts
    964
    Quote Originally Posted by oogabooga View Post
    The proper thing to do is to simply move the srand(time(NULL)) out of the loop. Put it in deck::deck(). In general, it should only be executed once in the entire run of the program. (So normally you'd also have a static bool to determine if it's already been done so that if another deck was created it won't be done again, but you don't need to do that here.)
    Why not just put it in main()? It doesn't make sense for deck to be responsible for seeding the RNG imo.
    How I need a drink, alcoholic in nature, after the heavy lectures involving quantum mechanics.

  9. #9
    Algorithm Dissector iMalc's Avatar
    Join Date
    Dec 2005
    Location
    New Zealand
    Posts
    6,308
    This is a clear sign that you are using the wrong loop type:
    Code:
        bool playAgain = true;
    
        //play game
        while(playAgain == true)
        {
    Instead, you should be using a do..while loop there.

    You also don't need the "== true" in there, as playAgain is already a boolean. Afterall, you wouldn't do this would you?:
    Code:
     while((playAgain == true) == true)
    My homepage
    Advice: Take only as directed - If symptoms persist, please see your debugger

    Linus Torvalds: "But it clearly is the only right way. The fact that everybody else does it some other way only means that they are wrong"

  10. #10
    C++ Witch laserlight's Avatar
    Join Date
    Oct 2003
    Location
    Singapore
    Posts
    22,258
    Quote Originally Posted by iMalc
    This is a clear sign that you are using the wrong loop type:
    Instead, you should be using a do..while loop there.
    A do while loop would be the best loop type here because it communicates the intention of running the loop at least once, but then a while loop isn't quite wrong either since it is good practice to initialise playAgain anyway, and it is thus easy to see that the loop will run at least once. However, if you were to modify this code to be in a function, say named askPlayAgain:
    Code:
    cout<<endl<<"Play Again? ";
    cin>>answer;
    if(answer != "yes") playAgain = false;
    Then you can dispense with the playAgain variable in main and have a neat:
    Code:
    do
    {
        // ...
    }
    while (askPlayAgain());
    C + C++ Compiler: MinGW port of GCC
    Version Control System: Bazaar

    Look up a C++ Reference and learn How To Ask Questions The Smart Way

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. segmentation fault
    By muppy in forum C Programming
    Replies: 10
    Last Post: 06-05-2011, 06:28 PM
  2. Fscanf segmentation fault issue (I think)
    By zouru in forum C Programming
    Replies: 5
    Last Post: 04-03-2011, 04:00 PM
  3. Segmentation fault
    By williamsonj in forum C Programming
    Replies: 36
    Last Post: 03-26-2010, 03:01 AM
  4. Segmentation fault issue
    By Stenland in forum C Programming
    Replies: 12
    Last Post: 09-19-2009, 05:56 AM
  5. segmentation fault and memory fault
    By Unregistered in forum C Programming
    Replies: 12
    Last Post: 04-02-2002, 11:09 PM

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21