Allocating memory by using Malloc (char & int)

Question

I'm new to C , and I'm trying to understand malloc. I'm trying to create a program that assigns memory for cards/colors and print them out.

I've made a function that looks like this:

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

#define ACE 1;
#define CardSize 52
#define colors 4

int main() {
    count();
    system("pause");
    return 0;
}

void count() {
    int *cards;
    int i, j, f;
    char *color[4] = { "Diamond", "Heart", "Spade", "Clubs"};
    cards = malloc(CardSize * sizeof(int));
    *color = malloc(colors * sizeof(char)); //Here's where my program crashes
    for (f = 0; f < 4; f++) {
        for (i = 0; i < 13; i++) {
            cards[i] = (i % 13) + 1;
            printf("%d of %s\n", cards[i], color[f]);
        }
    }
}

Without the line *color = malloc(colors*sizeof(char)); the program works fine, but I want to allocate memory for my colors.

The output is this:

1 of ════════════════════════════════════════════════════²²²²╬─Ép┐p
2 of ════════════════════════════════════════════════════²²²²╬─Ép┐p
3 of ════════════════════════════════════════════════════²²²²╬─Ép┐p
4 of ════════════════════════════════════════════════════²²²²╬─Ép┐p
5 of ════════════════════════════════════════════════════²²²²╬─Ép┐p
6 of ════════════════════════════════════════════════════²²²²╬─Ép┐p
7 of ════════════════════════════════════════════════════²²²²╬─Ép┐p
8 of ════════════════════════════════════════════════════²²²²╬─Ép┐p
9 of ════════════════════════════════════════════════════²²²²╬─Ép┐p
10 of ════════════════════════════════════════════════════²²²²╬─Ép┐p
11 of ════════════════════════════════════════════════════²²²²╬─Ép┐p
12 of ════════════════════════════════════════════════════²²²²╬─Ép┐p
13 of ════════════════════════════════════════════════════²²²²╬─Ép┐p

Which should be diamonds, then the rest is printed out fine, 1 of hearts, 2 of hearts, etc and all the other colors.

Please can you help me understand where I'm making the mistake, and what I'm doing wrong?

Answer 1

You define color to be an array of 4 pointers to char arrays that you initialize correctly:

char *color[4] = { "Diamon", "Heart", "Spade", "Clubs"};

But you immediately store another pointer in the first element of this array:

*color = malloc(colors*sizeof(char)); //Here's where my program crashes

You should not do that at all. I don't really understand your intention in doing so, but just remove this line and your program will behave correctly. Note that string literals are stored in read only memory, so color should really be defined as:

const char *color[4] = { "Diamon", "Heart", "Spade", "Clubs" };

Also fix the typo on Diamond, Hearts and Spades. Indeed the suits are plural except Diamond.

The value for card should probably be computed differently, as a number from 0 to 51. Also cards should be returned or freed if you do not use it. The code would be changed this way:

void count(void) {
    int *cards = malloc(CardSize * sizeof(int));
    int i, j, f;
    const char *color[4] = { "Diamond", "Hearts", "Spades", "Clubs" };

    for (i = 0; i < CardSize; i++) {
        j = (i % 13) + 1;  // Card value
        f = (i / 13) % 4;  // color number 0 to 3
        cards[i] = i % 52;
        printf("%d of %s\n", j, color[f]);
    }
    free(cards);
}

Answer 2

These extra allocations are for nothing but exercise, but try this.

void count() {
    int *cards;
    int i, j, f;
    const char *color_data[4] = { "Diamon", "Heart", "Spade", "Clubs"};
    char **color;
    cards = malloc(CardSize*sizeof(int));
    color = malloc(colors*sizeof(char*));
    for (i = 0; i < 4; i++) {
        color[i] = malloc((strlen(color_data[i]) + 1)*sizeof(char));
        strcpy(color[i], color_data[i]);
    }
    for (f = 0; f < 4; f++) {
        for (i = 0; i < 13; i++) {
            cards[i] = (i % 13) + 1;
            printf("%d of %s\n", cards[i], color[f]);
        }
    }
    free(cards);
    for (i = 0; i < 4; i++) {
        free(color[i]);
    }
    free(color);
}

Notes:

• Add #include <string.h> to the head of your code to use strlen() and strcpy().
• Omitted here, the return values of malloc() should be checked for not being NULL.
• Multiplying by sizeof(char) (=1) means nothing, but I kept it for not losing readability.
• Avoid memory leaks. Use free() for memories allocated via malloc().