C: accessing members of struct in a function

Question

I'm trying to use the struct member 'size' in my function print_shoe, but my for loop doesn't run. However, if I replace 'c->size' with an int in the for loop, it runs just fine

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#define DECK_SIZE 52
#define NUM_FACES 13
#define NUM_SUITS 4
#define LENGTH_FACES 6
#define LENGTH_SUITS 9

typedef struct cards {
  char suits[NUM_SUITS][LENGTH_SUITS];
  char faces[NUM_FACES][NUM_FACES];
  int suit, face, card, value, size;
  int *values[NUM_FACES];
} cards;

char buf[101];
void print_shoe();
void init_decks();
int rand_int();
void shuffle();

int main(void) {

  srand( time(NULL) );

  int decks_input = 0;    
  int numberOfDecks = 1;

  do {
    printf("\nEnter number of decks to be used in the game (1-8):\n\n");
    if (fgets(buf, sizeof(buf), stdin) != NULL)
      if (sscanf (buf, "%d", &decks_input))
        numberOfDecks = decks_input;
     } while (numberOfDecks < 1 || numberOfDecks > 8);

  cards *shoe = malloc(sizeof(cards) * numberOfDecks * DECK_SIZE);
  shoe->size = numberOfDecks * DECK_SIZE;

  shuffle(shoe);
  print_shoe(shoe);

  free(shoe);

  return 0;
}

void print_shoe(cards *c) {
  int i;
  for (i = 0; i < c->size; i++) {
    printf("card #%d = %s of %s\n", i+1, c->faces[c[i].face], c->suits[c[i].suit]);
  }
}

void init_decks(cards *c) {
  int i;
  for (i = 0; i < c->size; i++) {
    c[i].card = i;
    c[i].suit = c[i].card % NUM_SUITS;
    c[i].face = c[i].card % NUM_FACES;
  }  
}

void shuffle(cards *c) {
  init_decks(c);

  int i, j;
  cards tmp;
  for (i = c->size - 1; i > 0 ; i--) {
    j = rand_int(i + 1);
    tmp = c[j];
    c[j] = c[i];
    c[i] = tmp;
  }
}

int rand_int(int n) {
  int limit = RAND_MAX - RAND_MAX % n;
  int rnd;

  do {
    rnd = rand();
     } while (rnd >= limit);
  return rnd % n;
}

Edit: Question has been updated extensively in response to comments that it needed more clarification

Answer 1

Addressing the question directly and ignoring the wisdom of this approach, your problem is as follows(as also mentioned by Raymond Chen).

  cards *shoe = malloc(sizeof(cards) * numberOfDecks * DECK_SIZE);

The line above makes shoe point to enough memory to store (numberOfDecks * DECK_SIZE) struct cards. The structs, and thus there members, are all unitialized at this point meaning shoe[i].size could be any sequence of bits.

shoe->size = numberOfDecks * DECK_SIZE;

This line looks at the only the first struct cards and sets its size to (numberOfDecks * DECK_SIZE). The rest of the struct cards's size members remain unititialized.

In shuffle, your call to init_decks initializes card, suit, and face but not size. When you later shuffle the cards, a card with an unititialized size member has a good chance of becoming first.

Thus, under your current approach, the following should get what you want if you add this line to init_decks.

void init_decks(cards *c) {
      int i;
      int size = c->size;
      for (i = 0; i < c->size; i++) {
            c[i].size = size;
            c[i].card = i;
            c[i].suit = c[i].card % NUM_SUITS;
            c[i].face = c[i].card % NUM_FACES;
       }  
}

Answer 2

In the revised code, you have:

cards *shoe = malloc(sizeof(cards) * numberOfDecks * DECK_SIZE);
shoe->size = numberOfDecks * DECK_SIZE;

// You probably need init_decks(shoe); here!!!

shuffle(shoe);
print_shoe(shoe);

Your code in print_shoe() is simply printing, but you've not initialized the data from malloc() other than the size, so you're printing garbage. The data returned by malloc() is uninitialized and must be initialized before it is read. The question changed while I was typing; you still haven't called init_decks(shoe); as you need to.

---

This is not why it isn't working — I'm not sure what the problem is now — but it is almost worth commenting on. You have:

void shuffle(cards *c) {
  init_decks(c);

  int i, j;
  cards tmp;
  for (i = c->size - 1; i > 0 ; i--) {
    j = rand_int(i + 1);
    tmp = c[j];
    c[j] = c[i];
    c[i] = tmp;
  }
}

If you're going to use the C99 technique of declaring variables with minimal scope, then you should be writing:

void shuffle(cards *c) {
  init_decks(c);
  for (int i = c->size - 1; i > 0; i--) {
    int j = rand_int(i + 1);
    cards tmp = c[j];
    c[j] = c[i];
    c[i] = tmp;
  }
}

(I wouldn't have missed the call to init_decks() had it been written like that.)

---

As noted in a comment, your cards structure is rather top-heavy. You allocate (for each card) enough space to store the ranks and suits that it could possibly have. This is really not necessary.

This code separates a Deck from a Card. It uses a flexible array member in the Deck structure to hold the cards, which is convenient. You may prefer to use a regular pointer there, in which case you'll need a pair of memory allocations and a function deck_free() to release the memory allocated by deck_alloc().

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <time.h>

#define NUM_FACES 13
#define NUM_SUITS 4
#define DECK_SIZE (NUM_FACES * NUM_SUITS)
#define LENGTH_FACES 6
#define LENGTH_SUITS 9

static const char suits[NUM_SUITS][LENGTH_SUITS] =
{
    "Clubs",
    "Diamonds",
    "Hearts",
    "Spades"
};
static const char faces[NUM_FACES][NUM_FACES] =
{
    "Ace",
    "Deuce",
    "Three",
    "Four",
    "Five",
    "Six",
    "Seven",
    "Eight",
    "Nine",
    "Ten",
    "Jack",
    "Queen",
    "King",
};

typedef struct Card
{
  int suit;
  int face;
  int card;
} Card;

typedef struct Deck
{
  int size;
  Card cards[];     // Flexible array member
} Deck;

void print_shoe(const Deck *d);
void init_decks(Deck *d);
int rand_int(int n);
void shuffle(Deck *d);
static Deck *deck_alloc(int numberOfDecks);

int main(void)
{
    srand( time(NULL) );
    int numberOfDecks = 1;

    do
    {
        char buf[101];
        printf("\nEnter number of decks to be used in the game (1-8):\n\n");
        if (fgets(buf, sizeof(buf), stdin) != NULL)
        {
            int decks_input;    
            if (sscanf (buf, "%d", &decks_input))
                numberOfDecks = decks_input;
        }
    } while (numberOfDecks < 1 || numberOfDecks > 8);

    Deck *shoe = deck_alloc(numberOfDecks);
    shuffle(shoe);
    print_shoe(shoe);
    free(shoe);

    return 0;
}

static Deck *deck_alloc(int numberOfDecks)
{
    Deck *shoe  = malloc(sizeof(Deck) + (sizeof(Card) * numberOfDecks * DECK_SIZE));
    if (shoe == 0)
    {
        fprintf(stderr, "out of memory\n");
        exit(1);
    }
    shoe->size = numberOfDecks * DECK_SIZE;
    return shoe;
}

void print_shoe(const Deck *d)
{
    for (int i = 0; i < d->size; i++)
        printf("card #%d = %s of %s\n", i+1, faces[d->cards[i].face], suits[d->cards[i].suit]);
}

void init_decks(Deck *d)
{
    for (int i = 0; i < d->size; i++)
    {
        d->cards[i].card = i;
        d->cards[i].suit = d->cards[i].card % NUM_SUITS;
        d->cards[i].face = d->cards[i].card % NUM_FACES;
    }  
}

void shuffle(Deck *d)
{
    init_decks(d);
    for (int i = d->size - 1; i > 0 ; i--)
    {
        int j = rand_int(i + 1);
        Card tmp = d->cards[j];
        d->cards[j] = d->cards[i];
        d->cards[i] = tmp;
    }
}

int rand_int(int n)
{
    int limit = RAND_MAX - RAND_MAX % n;
    int rnd;

    do
    {
        rnd = rand();
    } while (rnd >= limit);
    return rnd % n;
}

Sample output:

$ ./cards

Enter number of decks to be used in the game (1-8):

1
card #1 = Eight of Clubs
card #2 = Jack of Clubs
card #3 = Deuce of Diamonds
card #4 = Jack of Hearts
card #5 = Queen of Clubs
card #6 = Four of Hearts
card #7 = Six of Spades
card #8 = King of Hearts
card #9 = Five of Spades
card #10 = King of Clubs
card #11 = Deuce of Clubs
card #12 = King of Spades
card #13 = Four of Spades
card #14 = Nine of Diamonds
card #15 = Five of Hearts
card #16 = Deuce of Spades
card #17 = Ten of Clubs
card #18 = Five of Diamonds
card #19 = Ten of Spades
card #20 = Three of Spades
card #21 = Nine of Hearts
card #22 = Six of Clubs
card #23 = Ace of Clubs
card #24 = Three of Clubs
card #25 = Queen of Hearts
card #26 = Jack of Diamonds
card #27 = Nine of Clubs
card #28 = Four of Clubs
card #29 = Seven of Spades
card #30 = Ace of Diamonds
card #31 = Six of Diamonds
card #32 = Three of Hearts
card #33 = Queen of Diamonds
card #34 = Ten of Hearts
card #35 = Ten of Diamonds
card #36 = Seven of Diamonds
card #37 = Seven of Clubs
card #38 = Deuce of Hearts
card #39 = Ace of Hearts
card #40 = Jack of Spades
card #41 = Eight of Diamonds
card #42 = Eight of Spades
card #43 = Ace of Spades
card #44 = Three of Diamonds
card #45 = Queen of Spades
card #46 = Five of Clubs
card #47 = Four of Diamonds
card #48 = King of Diamonds
card #49 = Nine of Spades
card #50 = Eight of Hearts
card #51 = Six of Hearts
card #52 = Seven of Hearts
$

Answer 3

You have declared a pointer but haven't initialized it a valid memory location to point to.

 ex *ex_p = malloc(sizeof(ex));
 ex_p->size = 10;