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

Are you freeing str, ever? How deep are you recursing in generateMoves?

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)

Thats the thing Adak, how do I free str if I need to return it in the function?

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?

The caller should be responsible for freeing it, in that case.

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.

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).

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?

Put a counter into GenerateMove(). It might be digging it's way to China.

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

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

Quote:

---

Originally Posted by Median

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.

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

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.