Don't understand why my code isn't working

[Simon]

I've got a simple and repetitive bit of code for a function in a C
implementation of the card game 31s I'm working on. BTW, I am a bit of
a novice at C; for the past couple of years I was using Visual Basic,
but I wanted portability, so I chose C.

I am developing this in XCode 2 (on Mac OS 10.4.7) as a normal C
command line utility; so this is basically just GCC. This code compiles
fine, without any errors or warnings but when I run it in XCode it
says: "31s has exited due to signal 11 (SIGSEGV)." and in Terminal it
says "Segmentation Fault". Here is the code of most of the function,
and what I'm pretty sure is the faulty bit. I can post all the code up
if needed; it's all in one .c file.

void print_stats() {
int
location_of_player_card_1,location_of_player_card_2,location_of_player_card_3;
int
location_of_CPU1_card_1,location_of_CPU1_card_2,location_of_CPU1_card_3;
int
location_of_CPU2_card_1,location_of_CPU2_card_2,location_of_CPU2_card_3;
int
location_of_middle_card_1,location_of_middle_card_2,location_of_middle_card_3;

int x,y;
while (x<52) {
if (where_are_cards[x] = PLAYER_HAND) {
switch (y) {
case 0:
location_of_player_card_1 = x;
y++;
break;
case 1:
location_of_player_card_2 = x;
y++;
break;
case 2:
location_of_player_card_3 = x;
y++;
break;
}
}
x++;
}

x,y = 0;

while (x<52) {
if (where_are_cards[x] = CPU1_HAND) {
switch (y) {
case 0:
location_of_CPU1_card_1 = x;
y++;
break;
case 1:
location_of_CPU1_card_2 = x;
y++;
break;
case 2:
location_of_CPU1_card_3 = x;
y++;
break;
}
}
x++;
}

x,y = 0;

while (x<52) {
if (where_are_cards[x] = CPU2_HAND) {
switch (y) {
case 0:
location_of_CPU2_card_1 = x;
y++;
break;
case 1:
location_of_CPU2_card_2 = x;
y++;
break;
case 2:
location_of_CPU2_card_3 = x;
y++;
break;
}
}
x++;
}

x,y = 0;

while (x<52) {
if (where_are_cards[x] = MIDDLE) {
switch (y) {
case 0:
location_of_middle_card_1 = x;
y++;
break;
case 1:
location_of_middle_card_2 = x;
y++;
break;
case 2:
location_of_middle_card_3 = x;
y++;
break;
}
}
x++;
}

printf("Table:\n");
printf(" Your hand: ");
print_card(pack[location_of_player_card_1]);
print_card(pack[location_of_player_card_2]);
print_card(pack[location_of_player_card_3]);

}

 

[Chris Dollin]

int x,y;
while (x<52) {

The value of `x` is indeterminate. Undefined behaviour. It
might, for example, be -1066 (assuming it's not a trap
value).

if (where_are_cards[x] = PLAYER_HAND) {

I'm /sure/ this isn't what you meant to say. You've assigned
`PLAYER_HAND` to `where_are_cards[x]`, and if PLAYER_HAND isn't
zero, you do the then-part (the switch below). I think you
meant to write `==`.

switch (y) {
case 0:
location_of_player_card_1 = x;
y++;
break;
case 1:
location_of_player_card_2 = x;
y++;
break;
case 2:
location_of_player_card_3 = x;
y++;
break;
}
}
x++;
}

Wouldn't it be better to have location_of_player_card being an
array so you could just write

location_of_player_card[y] = x;

(if necessary, checking y to be in range)?

x,y = 0;

I don't think that does what you think.

What it does is evaluate `x`, throw the value away, then assign 0
to `y`.

I suspect you need to write

x = y = 0;

while (x<52) {
if (where_are_cards[x] = CPU1_HAND) {
switch (y) {
case 0:
location_of_CPU1_card_1 = x;
y++;
break;
case 1:
location_of_CPU1_card_2 = x;
y++;
break;
case 2:
location_of_CPU1_card_3 = x;
y++;
break;
}
}
x++;
}

Almost exactly the same as the code above. Seek to remove duplication.

x,y = 0;

As above.

while (x<52) {
if (where_are_cards[x] = CPU2_HAND) {
switch (y) {
case 0:
location_of_CPU2_card_1 = x;
y++;
break;
case 1:
location_of_CPU2_card_2 = x;
y++;
break;
case 2:
location_of_CPU2_card_3 = x;
y++;
break;
}
}
x++;
}

As other above.

x,y = 0;
Ditto.

while (x<52) {
if (where_are_cards[x] = MIDDLE) {
switch (y) {
case 0:
location_of_middle_card_1 = x;
y++;
break;
case 1:
location_of_middle_card_2 = x;
y++;
break;
case 2:
location_of_middle_card_3 = x;
y++;
break;
}
}
x++;
}

Ditto ditto.

 

[Simon]

Thanks, Chris, this is really helpful. I've cleared up stuff you said
in long-hand just to make sure but it's still not quite working - my
program, that is. I've commented out a bit in shuffle_cards which
randomizes but it's still very wierd with all these "signal" errors.
Here's the complete code:

#include <stdio.h>
#include <stdlib.h> // for srand and rand
#include <time.h> // for time

typedef struct {
int suit; /* The suit of the playing card */
int value; /* The value of the playing card */
} CARD;

/* Set up an integer to represent each suit */
enum {CLUBS = 4, DIAMONDS = 3, HEARTS = 2, SPADES= 1};

/* Set up integers to represent non numeric cards */
enum {ACE = 14, JACK = 11, QUEEN = 12, KING = 13};


CARD pack[51];
int where_are_cards[51];
enum {PACK = 0, PLAYER_HAND = 1, CPU1_HAND = 2, CPU2_HAND = 3, MIDDLE =
4};


void print_card(CARD card_to_print) {
if ( card_to_print.value < 11 ) { printf("%d",card_to_print.value); }
if ( card_to_print.value == 11 ) { printf("Jack"); }
if ( card_to_print.value == 12 ) { printf("Queen"); }
if ( card_to_print.value == 13 ) { printf("King"); }
if ( card_to_print.value == 14 ) { printf("Ace"); }

printf(" of ");

if ( card_to_print.suit == 1 ) { printf("Spades"); }
if ( card_to_print.suit == 2 ) { printf("Hearts"); }
if ( card_to_print.suit == 3 ) { printf("Diamonds"); }
if ( card_to_print.suit == 4 ) { printf("Clubs"); }
printf("\n");
}

void shuffle_pack() {
srand(time(0)); // initialize seed "randomly"
int i,x,y,z = 0;

while (x < 4) {
while (y < 13) {
pack[x*y].value = y;
pack[x*y].suit = x;
y++;
}
y = 0;
x++;
}
/*
while (z<52) {
int r = rand() % 52; // generate a random position
CARD temp = pack; pack = pack[r]; pack[r] = temp;
z++;
}*/
}

void print_stats() {
int
location_of_player_card_1,location_of_player_card_2,location_of_player_card_3;
int
location_of_CPU1_card_1,location_of_CPU1_card_2,location_of_CPU1_card_3;
int
location_of_CPU2_card_1,location_of_CPU2_card_2,location_of_CPU2_card_3;
int
location_of_middle_card_1,location_of_middle_card_2,location_of_middle_card_3;

int x,y = 0;
while (x<52) {
if (where_are_cards[x] == PLAYER_HAND) {
switch (y) {
case 0:
location_of_player_card_1 = x;
y++;
break;
case 1:
location_of_player_card_2 = x;
y++;
break;
case 2:
location_of_player_card_3 = x;
y++;
break;
}
}
x++;
}

x = 0;
x = 0;

while (x<52) {
if (where_are_cards[x] == CPU1_HAND) {
switch (y) {
case 0:
location_of_CPU1_card_1 = x;
y++;
break;
case 1:
location_of_CPU1_card_2 = x;
y++;
break;
case 2:
location_of_CPU1_card_3 = x;
y++;
break;
}
}
x++;
}

x = 0;
x = 0;

while (x<52) {
if (where_are_cards[x] == CPU2_HAND) {
switch (y) {
case 0:
location_of_CPU2_card_1 = x;
y++;
break;
case 1:
location_of_CPU2_card_2 = x;
y++;
break;
case 2:
location_of_CPU2_card_3 = x;
y++;
break;
}
}
x++;
}

x = 0;
x = 0;

while (x<52) {
if (where_are_cards[x] == MIDDLE) {
switch (y) {
case 0:
location_of_middle_card_1 = x;
y++;
break;
case 1:
location_of_middle_card_2 = x;
y++;
break;
case 2:
location_of_middle_card_3 = x;
y++;
break;
}
}
x++;
}

printf("Table:\n");
printf(" Your hand: "); print_card(pack[location_of_player_card_1]);
printf(" "); print_card(pack[location_of_player_card_2]);
printf(" "); print_card(pack[location_of_player_card_3]);

}

void play_game() {
printf("\nYou will be playing against two computer players.\n");
printf("Shuffling pack... "); shuffle_pack(); printf("Done!\n");

int i=0; while (i<52) { where_are_cards = PACK; i++; } //
Initialize where_are_cards[51] array
i=0; while (i<3) { where_are_cards = PLAYER_HAND; i++; } // Give
player 3 cards
i=3; while (i<3) { where_are_cards = CPU1_HAND; i++; } // Give CPU1
3 cards
i=6; while (i<3) { where_are_cards = CPU2_HAND; i++; } // Give CPU2
3 cards
i=9; while (i<3) { where_are_cards = MIDDLE; i++; } // Put 3 cards
in the middle

print_card(pack[5]);

print_stats();
}

int main (int argc, const char * argv[]) {
printf("Welcome to the game of 31s!\n");
printf("This was created by Simon Goldring in 2006.\n");

int menu;
while ( menu != 1 ) {
printf("\nMAIN MENU:\n");
printf(" 0 - Play a game of 31s with the computer\n");
printf(" 1 - Quit 31s\n");
printf("? ");
scanf("%d", &menu);
if ( menu == 0 ) {
play_game();
}
}

printf("\nThanks for playing!");

return 0;
}

 

[Chris Dollin]

Thanks, Chris, this is really helpful. I've cleared up stuff you said
in long-hand just to make sure but it's still not quite working - my
program, that is. I've commented out a bit in shuffle_cards which
randomizes but it's still very wierd with all these "signal" errors.

Those are likely array-indexing errors. (You don't get a nice message
when that happens in C - things will go loudly wrong if you're lucky,
quietly wrong if you're unlucky.)

You should read a decent introductory C book - your previous language
is interfering and you need a bit of reprogramming!

Just a few quick notes at days end:

void shuffle_pack() {
srand(time(0)); // initialize seed "randomly"
int i,x,y,z = 0;

Only initialises `z`. `i`, `x`, and `y` are all junk.

int i = 0, x = 0, y = 0, z = 0;
or
int i = 0;
int x = 0;
int y = 0;
int z = 0;

int x,y = 0;
Ditto.


x = 0;
x = 0;

One of those should be `y = 0`?

x = y = 0;

x = 0;
x = 0;

Ditto.

x = 0;
x = 0;

Ditto.

 

[Mark McIntyre]

Thanks, Chris, this is really helpful. I've cleared up stuff you said
in long-hand just to make sure but it's still not quite working - my
program, that is. I've commented out a bit in shuffle_cards which
randomizes but it's still very wierd with all these "signal" errors.
Here's the complete code:
int i,x,y,z = 0;

This only initialises z to zero. Same problem appears several other
times.

Top tip: do not declare more than one variable on a line;
int i = 0;
int x = 0;
that way, you cannot make this sort of mistake.
(or this one:
FILE *a, b;
)


By the way, while some C compilers now allow you to declare variables
where used, most do not - you must declare all variables at the start
of a block. I worry that you are accidentally compiling this code as
C++.
--
Mark McIntyre

"Debugging is twice as hard as writing the code in the first place.
Therefore, if you write the code as cleverly as possible, you are,
by definition, not smart enough to debug it."
--Brian Kernighan

 

[Kenneth Brody]

Simon wrote:
[...]

CARD pack[51];
int where_are_cards[51];

[...]

Last I checked, there are 52 cards in the deck.

CARD pack[52];
int where_are_cards[52];

int i,x,y,z = 0;

i, x, and y are not initialized here.

int i=0, x=0, y=0, z=0;

while (x < 4) {
while (y < 13) {
pack[x*y].value = y;
pack[x*y].suit = x;
y++;
}
y = 0;
x++;
}

You probably want "pack[x*13+y]" here.

--
+-------------------------+--------------------+-----------------------+
| Kenneth J. Brody | www.hvcomputer.com | #include |
| kenbrody/at\spamcop.net | www.fptech.com | <std_disclaimer.h> |
+-------------------------+--------------------+-----------------------+
Don't e-mail me at: <mailto:[email protected]>

 

[Simon]

I'm changing all the initializing stuff, but I don't understand one
thing; doesn't an array tart at 0, so 52 items would be 0-51?

Thanks for all the help.

 

[Richard Heathfield]

Simon said:

I'm changing all the initializing stuff, but I don't understand one
thing; doesn't an array tart at 0, so 52 items would be 0-51?

Sure, 52 items would be 0-51, but if you define it as being CARD pack[51],
that's 51 items, 0-50. In an array definition, the number between [ and ]
is the number of objects you want the array to contain, not the number of
the highest index in the array.

 

[Simon]

aha! Cheers!

 

[Default User]

aha! Cheers!

Please quote enough of the previous message for context. Google does
this automatically now, so you have no excuse.




Brian