Thread: Segmentation fault: I am losing my mind :(

  1. #1
    Registered User
    Join Date
    Sep 2012
    Posts
    43

    Segmentation fault: I am losing my mind :(

    Hello!

    I am programming the 4 in line game, and I've been getting a segmentation fault error that I'm not being able to correct. I've read dozens of posts and still didn't get it.

    Please bare with me with this problem.

    I've been able to narrow the problem to the following routine:

    After human player makes move, computer generates random move. For each random move it generates, the program checks if the move is valid (If there are no empty cells bellow). For him to do that, I made a function that converts the column in question to a string.

    Here is the code of the 3 functions.

    Code:
    char * ColGameToStr (jogo_4line * jg, int numcol)
    {
      int i, n;
      
      i=numcol;
     
      if (i>47 && i<56) i = numcol-48-1;
    
    
    
      char *str = malloc (sizeof (char) * 7);
      if(!str)
      {return NULL; 
      exit(0);}
      
      for (n = 0; n < 6; n++)
        {
          str[n] = jg->T[n][i];
    
        }
        
      
      str[n] = '\0';
    
      return(str);
    
    }

    Code:
    int CheckMove (char * move, int player, jogo_4line * jg)
    {
      char * colstr = malloc(sizeof(char)*6+1);
      char *colaux=colstr;
      int n;
      
      
      
      colaux=ColGameToStr(jg,move[2]);
      colaux[7]='\0';
    
      
      for (n=move[0]-48-1-1; n>=0;n--)
      { 
        if (colaux[n]==EMPTY)
        { free(colstr);
          return 0;
          
        }
          
        }
    
      free(colstr);
      return 1;
    
    }

    Code:
    void GenerateMove (jogo_4line * jg)
    {
      char line, col;
      
      srand48 (time (NULL));
      
      line=drand48()*6+49;
      col=drand48()*7+49;
      char play[]={line,',',col};
    
    
      
      if (jg->T[line-1-48][col-1-48]==EMPTY && CheckMove(play, jg->turn, jg))
      {
        jg->T[line-48-1][col-48-1]=COMPUTER;
        jg->turn=1;
           
      }
      else GenerateMove(jg);

    Additional information:

    jogo_4line is the following struct:

    Code:
    typedef struct {cell T[6][7]; int turn; int endgame;} jogo_4line;
    The T[6][7] represents each cell of the board.

    COMPUTER and EMPTY are macros that represent a state of a cell. EMPTY=' ', COMPUTER='X' .

    When GenerateMove happens to find a valid move quickly, the segmentation fault doesnt happen, only if the functon GenerateMove calls itself a certain amount of times.

    I think the problem is in the ColGameToStr function, but I'm really unable to find a solution for this

  2. #2
    Registered User
    Join Date
    Sep 2006
    Posts
    8,868
    Are you freeing str, ever? How deep are you recursing in generateMoves?
    Last edited by Adak; 10-30-2012 at 09:20 AM.

  3. #3
    Registered User
    Join Date
    Sep 2012
    Posts
    43
    If I let the computer play wherever he wants, meaning, if the program doesn't verify the move with CheckMove, I don't get segmentation fault (core dumped)

  4. #4
    Registered User
    Join Date
    Sep 2012
    Posts
    43
    Thats the thing Adak, how do I free str if I need to return it in the function?

  5. #5
    Registered User
    Join Date
    Sep 2006
    Posts
    8,868
    When you're done with it, free it. Because the CheckMove function calls the ColtoStr, so many times.

    Do you use Valgrind or any memory checker tool?
    Last edited by Adak; 10-30-2012 at 09:24 AM.

  6. #6
    C++ Witch laserlight's Avatar
    Join Date
    Oct 2003
    Location
    Singapore
    Posts
    28,413
    The caller should be responsible for freeing it, in that case.
    Quote Originally Posted by Bjarne Stroustrup (2000-10-14)
    I get maybe two dozen requests for help with some sort of programming or design problem every day. Most have more sense than to send me hundreds of lines of code. If they do, I ask them to find the smallest example that exhibits the problem and send me that. Mostly, they then find the error themselves. "Finding the smallest program that demonstrates the error" is a powerful debugging tool.
    Look up a C++ Reference and learn How To Ask Questions The Smart Way

  7. #7
    Registered User
    Join Date
    Sep 2012
    Posts
    43
    If that is the case, I need to free str in the CheckMove function.

    So the next question is (I am sorry if this is basic, but im newbie in C): How do I free a variable in a function other than the one it is declared in?

    @Adak: No. I'm not using such tools, I dont know how to do it.

  8. #8
    C++ Witch laserlight's Avatar
    Join Date
    Oct 2003
    Location
    Singapore
    Posts
    28,413
    Quote Originally Posted by Median
    How do I free a variable in a function other than the one it is declared in?
    Since you save the return value in colaux, you just call free(colaux).
    Quote Originally Posted by Bjarne Stroustrup (2000-10-14)
    I get maybe two dozen requests for help with some sort of programming or design problem every day. Most have more sense than to send me hundreds of lines of code. If they do, I ask them to find the smallest example that exhibits the problem and send me that. Mostly, they then find the error themselves. "Finding the smallest program that demonstrates the error" is a powerful debugging tool.
    Look up a C++ Reference and learn How To Ask Questions The Smart Way

  9. #9
    Registered User
    Join Date
    Sep 2012
    Posts
    43
    Thank you for your answers

    @laserlight

    I freed colaux in the CheckMove function after the for cycle, but the problem persists. It segfaults the moment I need the computer to make a move. I am really lost.

    You guys don't see nothing else that might be causing this?

  10. #10
    Registered User
    Join Date
    Sep 2006
    Posts
    8,868
    Put a counter into GenerateMove(). It might be digging it's way to China.

  11. #11
    Registered User
    Join Date
    Sep 2012
    Posts
    43
    OK I solved the problem this way:

    Code:
     void GenerateMove (jogo_4line * jg)
    {
      char line, col;
      int cond=TRUE;
        
      while (cond==TRUE)
      {
        srand48 (time (NULL));
      
      line=drand48()*6+49;
      col=drand48()*7+49;
      char play[]={line,',',col};
    
    
      
      if (jg->T[line-1-48][col-1-48]==VAZIO && CheckMove(play, jg->vez, jg))
      {
        jg->T[line-48-1][col-48-1]=JOGADOR2;
        jg->vez=1;
        cond=FALSE;
           
      }
      }
    
    
    }
    I traded the self-calling of GenerateMove for a while cycle and the segfault doesn't happen anymore.

    Some times it takes 3 or 4 seconds to generate a move which is weird.. I thought it would be quicker. I didn't put a counter but obviously it tries his way to china b4 finding a valid move.

    This made me even more curious as to why my other solution wouldn't work

  12. #12
    Registered User
    Join Date
    Sep 2012
    Posts
    43
    Some times it takes up to 10 seconds, something is not right. I have an intel i7 which kills in calcs per second. I dont get it

  13. #13
    Registered User
    Join Date
    Sep 2006
    Posts
    8,868
    Quote Originally Posted by Median View Post
    Some times it takes up to 10 seconds, something is not right. I have an intel i7 which kills in calcs per second. I dont get it
    I haven't worked with 4 in a row, but I know JUST who you can talk to:

    <big evil grin>

    This is like chess programming, but simpler (no enpassant moves,etc. ), but the best chess programmers are on this forum.
    TalkChess.com :: Index
    Post in the "Programming and Technical Discussions" forum. (last one)

    Never mind the screaming -- they have issues with clones, like you wouldn't believe.

    They will get you sorted out.

    Yes, I'm very familiar with the i7, and you are quite correct, something's definitely wrong.

  14. #14
    Registered User
    Join Date
    May 2012
    Posts
    1,066
    Code:
    while (cond==TRUE)
    {
        srand48 (time (NULL));
    Move the srand48() out of your loop. You should call the seed initialization only once in your program.
    In your case, you call it at every iteration. But time() returns the time in seconds. Thus one second long you get the same time and this means you start the random generator with the same seed and you get every time the same pseudorandom numbers from drand48(). If the calculated position is not valid, your loop is for one second practically doing nothing.
    For illustration look at the output of this program:
    Code:
    #include <stdio.h>
    #include <stdlib.h>
    #include <time.h>
    
    
    int main(void)
    {
        int i;
        for (i = 0; i < 20; i++)
        {
            srand48(time(NULL));
            printf("%f\n", drand48());
        }
        return 0;
    }
    How about another approach:
    Make a temporary array holding the numbers from 1 to 7 (7 columns), shuffle it (https://en.wikipedia.org/wiki/Fisher...3Yates_shuffle) and then go through the array from the beginning until you get the first valid column. Thus you have 14 iterations at the worst case (7 iterations for the shuffling, another 7 if the only valid column happens to be in the last position of the array).

    Bye, Andreas

  15. #15
    and the hat of int overfl Salem's Avatar
    Join Date
    Aug 2001
    Location
    The edge of the known universe
    Posts
    39,660
    Code:
      char *str = malloc (sizeof (char) * 7);
      if(!str)
      {return NULL;
      exit(0);}
    Did your compiler warn about unreachable code?
    If it returns NULL, you're not going to exit()

    Code:
      char * colstr = malloc(sizeof(char)*6+1);
      char *colaux=colstr;
      int n;
    
      colaux=ColGameToStr(jg,move[2]);
      colaux[7]='\0';
    1. Well the initial assignment is a waste of time, since you just overwrite it with another value anyway.
    2. ColGameToStr() only does malloc(7), so the subscript here is out of bounds (this is very bad)
    3. colaux is pointing at allocated memory, but you never free it
    4. ColGameToStr() can return NULL, but you never check to see that it is here.

    All those 6's and 7's need to be replaced by some suitable #define CONSTANT to make the code easier to read and maintain.
    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.

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Pipelines - please help, 6 hours and I am losing my mind.
    By Johnny_010 in forum C Programming
    Replies: 4
    Last Post: 04-04-2012, 02:06 PM
  2. Replies: 6
    Last Post: 03-18-2012, 10:11 PM
  3. Array of struct pointers - Losing my mind
    By drucillica in forum C Programming
    Replies: 5
    Last Post: 11-12-2005, 11:50 PM
  4. Don't know if I'm losing my mind...
    By Invincible in forum C++ Programming
    Replies: 19
    Last Post: 05-26-2002, 11:27 PM
  5. Lost source... losing mind...
    By doubleanti in forum A Brief History of Cprogramming.com
    Replies: 6
    Last Post: 10-27-2001, 12:31 AM