Seg fault

Hi.

I am getting a Seg fault and I don't know why.

Basically, in blackjack, I have a hand of cards and I am trying to sum up the hand:

Code:

struct card_t {
	unsigned short int card;  /* Ace-King (1-13) */
	unsigned short int suit;  /* Clubs,Diamonds,Hearts,Spades (1-4) */
};

int main(void) {
  ....

  printf("mycards score = %d\ncompscards score = %d\n", sum_up_hand( mycards ), sum_up_hand( compscards ) );

 ......
}

unsigned short int 
sum_up_hand( struct card_t *hand ) {

	int i = 0;
	unsigned short int total = 0;
	while( (hand + i) != NULL ) {
		total += value_of_card( (hand + i)->card );
		i++;
	}
	
	return total;

}

unsigned short int
value_of_card( unsigned short int n ) {

	int i, value = 0;
	for(i = 0; i < 10; i++) {
		/* card ace corresponds to i=0, card 2 corresponds to i=1, and so on until i=9*/
		value = ( n == i ) ? i + 1 : 0;
	}

	switch( n ) {
	
		case 10: //jack
		  value = 10;
		  break;
		case 11: // queen
		  value = 10;
		  break;
		case 12:  //king
		  value = 10;
		  break;

	}

	return value;

}

Above is just the necessary code, well hopefully. Compiles ok, I go to run it and receive a Seg fault. Any idea? I know I'm trying to access something which doesn't belong to me, but I don't know what.

> while( (hand + i) != NULL )
Well this isn't going to be true for a long while, not until the address wraps round to NULL.
In the mean time, any invalid address will segfault.

How about a for loop which counts through at most 52 cards?

Also, using %d to print an unsigned short is also problematic.

Hmm I'm not sure about the for loop. Seems unnecessary to loop more than is needed. There must be a way to stop when it comes to an uninitialised value?

Code:

for(i=0;i<52;i++) {
		total += value_of_card( (hand + i)->card );
}

Wont I get garbage for 'total', as memory is filled with garbage initially? I suppose I could set every value to -1 and loop until it hits a -1? (And remove the unsigned obviously). What would you suggest?

About the %d, you're right; I couldn't find a flag for short, so assuming there isn't one? Will I just cast (int) to the argument then in that case?

%hd is for short.

And as for checking for invalid, how about

Code:

struct card_t {
        unsigned char isValid;  /* 1 is valid, 0 is not */
	unsigned short int card;  /* Ace-King (1-13) */
	unsigned short int suit;  /* Clubs,Diamonds,Hearts,Spades (1-4) */
};

Or if hands are always the same size, maybe
sum_up_hand( struct card_t *hand, size_t numCardsInHand )

I guess what you mean was

Code:

while( *(hand + i) != NULL ) // same as hand[i] != NULL

Better way to implement your function would be to add an extra argument telling the number of element to check

Code:

unsigned short int 
sum_up_hand(struct card_t *hand, int nbOfCards )

There's also different ways of doing this...


And your function value_of_card seems overly complicated. Also, i don't know if you realized that this loop

Code:

for(i = 0; i < 10; i++)
{
	value = ( n == i ) ? i + 1 : 0;
}

was in fact equivalent to this

Code:

value = (n == 9) ? 10 : 0;
i = 10;

As for this

Code:

switch( n ) {
	
		case 10: //jack
		  value = 10;
		  break;
		case 11: // queen
		  value = 10;
		  break;
		case 12:  //king
		  value = 10;
		  break;

	}

could be replaced with this

Code:

if (n >= 10)
	value = 10;

And by the way, the format specifier for unsigned variable is %u, whatever it's an unsigned char, short or int. But i'm not sure this would have lead to incorrect output, since an unsigned short promoted to an (signed/unsigned) int wouldn't have mess with the sign bit of the int, in the case int is of larger size than short. But i'm not 100% sure.

Oh and by the way there's a difference between an "uninitialised value" an the NULL value. If one variable has the NULL value it is because it has been initialised to the NULL value.

Sorry, I did make a fair amount of foolish mistakes. New functions:

Code:

unsigned short int 
sum_up_hand( struct card_t *hand , unsigned short int no_of_cards ) {

	int i;
	unsigned short int total = 0;

	for(i = 0 ;i < no_of_cards;i++) {
		total += value_of_card( (hand + i)->card );
	}
	
	return total;

}

unsigned short int
value_of_card( unsigned short int n ) {

	int value = 0;
	if( n < 10 ) {
		value = n+1;
	}

	if( n >= 10 )
		value = 10;

	return value;

}

Code:

while( *(hand + i) != NULL ) // same as hand[i] != NULL

That is indeed what i meant. D'oh.

Code:

for(i = 0; i < 10; i++)
{
	value = ( n == i ) ? i + 1 : 0;
}

Yes, i see it now, thanks. Pesky 0.

%hd is for short.

Thanks, I missed that.

Thank you foxman and Salem, much appreciated.

Originally Posted by foxman

And by the way, the format specifier for unsigned variable is %u, whatever it's an unsigned char, short or int. But i'm not sure this would have lead to incorrect output, since an unsigned short promoted to an (signed/unsigned) int wouldn't have mess with the sign bit of the int, in the case int is of larger size than short. But i'm not 100% sure.

Code:

int printf(const char * format, ... );

it wouldn't be cast because the compiler wouldn't know what to cast it to.

Well i wasn't talking about cast, but more about promotion, because, like you mentionned, the called function doesn't know the type of the argument so i tought C was maybe putting in place the char->short->int and float->double promotion in this case.

Because i'm sure you already saw those kind of code working perfectly well

Code:

char c = 'a';
unsigned int i = 0xFF0000FF;

printf("ASCII code of 'a' is %d, and i have %u things", c, i);

But that's it, i don't know exactly how are working function with varying number of additional arguments (even if i have some ideas). I looked a bit on the net but found nothing really explicit. I still found this, who let me believe i'm right. That's it, promotion is happening in those case, for variable of type char, short (to int) or float (to double).

the thing about variable argument lists is that on type information is not described in it to the compiler. so it cannot perform promotion on those variables. the printf function does it internally, with casts from the data provided in the char * format string. Without that, it would be completely clueless. and easy to get wrong. so compilers sometimes have extra checks for the crt functions that use vargs so that you don't trip up so often. but other than that, it's completely clueless. in fact, it can ........ things up.

Code:

#include <stdio.h>

int main(void)
{
	printf("&#37;c ", 10000000);

	return 0;
}

Code:

C:\>gcc -std=c99 -Wall -Werror -pedantic main.c

C:\>a
Ç
C:\>

edit: I just remembered you were talking about promotion, not the other way around, but the idea is the same. the compiler has to know what the types are to do promotion.

edit2:

9 If a conversion specification is invalid, the behavior is undefined.242) If any argument is
not the correct type for the corresponding conversion specification, the behavior is
undefined.

What i meaned is that there is promotion BEFORE the function (the one with the variable arguments list) is called, and it's the "promoted" version of the values who are passed to the function. The compiler does know the type of the arguments you are passing . But the called function won't know.

Well, for things to be clear, what i mean is that if you write something like this

Code:

char c = 'a';
printf("%d", c);

well the compiler will add some instructions (promoting char to int) so it's exactly like you wrote this

Code:

char c = 'a';
printf("%d", (int) c);

how would it know you want it converted to an int?

"the usual arithmetic conversions"

[edit]Oops. I meant, default argument promotions (see 6.5.2.2p7).

oh ok, I guess I was wrong.
thanks Dave_Sinkula.

Sorry to digress, but I completed the blackjack game. Any advice/suggestions/improvements would be appreciated. Thanks