Segmentation fault: I am losing my mind :(

Hey, thank you all for your answers.

@ AndiPertsi

That is a pretty nice solution! I'll try to implement it and then ill give you updates.

@ Salem You are right, my code is a complete mess, I'm this close to doing everything all over again, but unfortunatly I have no time for that. I have to organize it a little bit... I think if you saw the whole thing you would gasp :P

Did your compiler warn about unreachable code?
If it returns NULL, you're not going to exit()

You are right, but strangely the compiler didnt complain.

And if I exit(0) first? Will it return NULL?

Originally Posted by Median

@ Salem You are right, my code is a complete mess, I'm this close to doing everything all over again, but unfortunatly I have no time for that. I have to organize it a little bit... I think if you saw the whole thing you would gasp :P

I spent some minutes trying to understand what you do and frankly I don't even want to look at the rest of your code. Are you practicing for IOCCC?

Seriously, why do you use ASCII values for the number values, i.e. use '1' (a char) instead of 1 (an int)?

And actually you don't really need three functions for deciding if a move is correct. You don't use any sort of AI for the computer, so just get a random column and put the computer chip on the next free line of this column (the first line from the bottom which is empty). Only if the column is already filled up you have to get another random column.

Bye, Andreas

That was arsh ... I started programming in C 1 month ago, cut me some slack, I will get better with practice

> And if I exit(0) first? Will it return NULL?
exit() does not return.

It's "game over" time - the program just ends.

Originally Posted by Median

That was arsh ... I started programming in C 1 month ago, cut me some slack, I will get better with practice

I'm sorry if my critique was too harsh. I don't want to discourage you.

But the way you use ASCII values for passing around numbers shows IMHO a fundamental misapprehension of the basic data types.

Is C your first programming language? What learning resources are you using?

Bye, Andreas

Originally Posted by AndiPersti

I'm sorry if my critique was too harsh. I don't want to discourage you.

But the way you use ASCII values for passing around numbers shows IMHO a fundamental misapprehension of the basic data types.

Is C your first programming language? What learning resources are you using?

Bye, Andreas

This is my first program that has more than 40 lines.

I'm having a class in college that is being my main resource for learning. My teacher gave us the basis of the 4 in line program which was the struct definition we should use.

Since the cell is of type char, and the compiler complains if I use char as indexes for arrays, I use integer values and then subtract the value read in by the keyboard (which is a char) for '0' (48). That way the compiler didn't complain anymore.

What do you think is my main misconception? I appreciate any critiques.

Originally Posted by Median

Since the cell is of type char, and the compiler complains if I use char as indexes for arrays,

Honestly, I wasn't aware of that warning (I've never got it before). Here's a good explanation why you've got it (just read through all 6 messages).

Originally Posted by Median

I use integer values and then subtract the value read in by the keyboard (which is a char) for '0' (48). That way the compiler didn't complain anymore.

The reason why the compiler doesn't complain is rather subtle.
You declare "line" and "col" as char inside "GenerateMove()", thus if you would use both variables like

Code:

line=drand48()*6+49;
col=drand48()*7+49;
jg->T[line][col]=JOGADOR2;

you would still get the warning because "line" and "col" are of type char and thus your indices would be char values.

But instead you use them like

Code:

line=drand48()*6+49;
col=drand48()*7+49;
jg->T[line - 48 - 1][col - 48 - 1]=JOGADOR2;

Here, your array indices are expressions ("line - 48 - 1" and "col - 48 - 1") where char types ("line" and "col") and integer types (48, 49, 1 - a decimal constant written in your source code is usually of type int (see footnote below)) are mixed. Thus the result of the expression is of type int because the char type is promoted to type int. Now your indices are of type int and that's why the compiler doesn't complain.

Originally Posted by Median

What do you think is my main misconception?

You don't need to read the input as a char. Read it as an integer. Then you avoid adding/subtracting 48 everytime (BTW: instead of using the magic number, you should add/subtract '0' everytime you need to convert the ASCII-value of a number to its numeric value).

Generally, char is just another integer type. Internally the character you get is stored as value according to your systems character encoding. That's why you can calculate with characters because the compiler uses this value.
But the values of a char variable aren't restricted to be always interpreted as characters. In your case, you want to calculate two random indices for the computer move. So you could just calculate a random value for "line" which is between 0 and 5 and use it at a legal index. Although "line" is still of type char you aren't interested in the character this value would represent if interpreted as a ASCII code. You just want to use the value.

Bye, Andreas

Footnote: If the value of the constant doesn't fit into the size of an int, it is of type long int or even unsigned long int. These are the rules for C90. The order in C99/C11 is int, long int, long long int.
You can also specify a type to the constant by suffixes:
1 is a constant of type int
1u or 1U is a constant of type unsigned int
1l or 1L is a constant of type long int
1ul or 1UL is a constant of type unsigned long int
C99/C11 add the suffixes ll respectively LL for long long int.

@AntiPersi!

Thank you so much for taking the time to write such an extensive answer, I really appretiate it.

I thought about that, reading the move as an integer, it sure would avoid all this meaningless subtractions, etc. But since the move had to be written in the following way [row,col], i didn't know how the compiler would react to the comma, so I decided to scan everything as string and then treat it accordingly.

One of the first things I learnt about C programming (and all programming in general) is that everything is about numbers, nothing about letters. However, I realize that the way I turn chars into integers isn't pretty at all.

Regards

Originally Posted by Median

I thought about that, reading the move as an integer, it sure would avoid all this meaningless subtractions, etc. But since the move had to be written in the following way [row,col], i didn't know how the compiler would react to the comma, so I decided to scan everything as string and then treat it accordingly.

I guess you are speaking of the user input here.
In that case, reading everything as a string is actually a good approach. But as soon as you got the string, you should extract the necessary information. You are only interested in the row and column. You don't need the separating comma further on, so there's no reason to pass it around to other functions.
If you think more about the user input you should realize that a player can only choose the column where s/he wants to drop the next disc. So you only need the column. (Assuming we talk about the same Four in a Line game)

You could also clean up your function interfaces. For example, your CheckMove() expects three parameters: a pointer to char called "move", an int "player" and a pointer to the "jogo_4line" struct. But you never use "player". And even if you would need this information, you have access to it using the "turn" member of your struct.

Bye, Andreas

You are right.

I rebuilt my whole program in order to make it simpler. This time i accepting one digit (integer) as user input. It simplifies everything.

But another problem arises. When I ask user for input, if user inputs anything other than a number the program malfunctions. How can i prevent that?

Cheers

Originally Posted by Median

You are right.

I rebuilt my whole program in order to make it simpler. This time i accepting one digit (integer) as user input. It simplifies everything.

But another problem arises. When I ask user for input, if user inputs anything other than a number the program malfunctions. How can i prevent that?

Cheers

Add this, after you ask for input:
getchar();

or

add a single space before the % in your scanf():
scanf(" %d", &yourChoice);

hehe it did the trick

This is the last thing I have to implement in my code and the game will be completed.

I need to be able to save and load game. For that I save the values of the cells in a txt file in the following manner:

Code:

int SaveGame(jogo_4line *jg)
{
  FILE *fp;
  int n;
  char *lines[MAXLINES];
  fp=fopen("SavedGame","w");
    if (!fp) 
  {
    printf("Error creating file\n");
    return 0;
  }
   for (n = 0; n < MAXLINES; n++)
    {
      lines[n] = LinGameToStr (jg, n);
      fprintf(fp,"%s\n",lines[n]);
    } 

  fclose(fp);
  return 1;
}

The SavedGame file could look like this (ofc this would be impossible but you get the point)

XXOOOXXX
XXOOOXXX
XXOOOXXX
XXOOOXXX
XXOOOXXX
XXOOOXXX

But I having a hard time in the loadgame function:

Code:

int LoadGame( jogo_4line *jg)

{
  int n=0;
  FILE *fp;
  char *lines=malloc(8);
  fp=fopen("SavedGame","r");
  if (!fp) 
  {
    printf("File not found\n");
    return 0;
  }
  
  while(fscanf(fp,"%s",lines)==1) 
  { printf("%d - %s\n",n,lines);
    n++;
  }
  
  free(lines);
  fclose(fp);
  return 1;
}

(I do a printf in the while cycle to inspect the output)

The problem is, when the fscanf finds a SPACE (cell where no one has played it), it stops reading and assigns the rest of the line (after the space) to a new line.

So, if SavedGame file looks like this,

line 0 - OOXOXOO
line 1 - XXXO XX
line 2 - XOOO
line 3 - O XX
line 4 -
line 5 -

The printf output will be

0 - OOXOXOO
1 - XXXO
2 - XX
3 - XOOO
4 - O
5 - XX

And this is no good. How can I get around this?

Here is a printscreen of the situation I described here to make it easier for you guys to understand: