Custom card library and blackjack demo

I am new to C and wanted to get feedback on a small project I was working on. Coming from python, I quickly learned that a lot of things we take for granted like list class and all of its useful functions are not implemented. I wanted to see if I could make a card library to help me build multiple types of card games. I just created a demo Blackjack game for example (Yes Ace is only counted as 1 for the demo) to show how someone would use it. If possible, can you let me know what I have done well (if any), what I can improve on and any other information that is helpful?

below is card.h

//card.h
#ifndef CARD_H
#define CARD_H

typedef struct Card Card;
typedef struct Deck_node Deck_node;
typedef struct Deck Deck;

struct Card{
    int suit;
    int number;
    int face;
    char* symbol;
};

struct Deck_node{
    Deck_node* prev;
    Card* card;
    Deck_node* next;
};

struct Deck{
    Deck_node* head;
    Deck_node* tail;
    int size;
};

Card *card_create(int suit, int number, char *symbol, int symbol_length);

Deck_node* deck_node_add_next(Deck_node *deck_node, Deck_node *next); 

Deck_node *deck_node_create(Card *card); 

void deck_node_destroy(Deck_node *deck_node);

Deck *deck_create();

void deck_clear(Deck *deck);

void deck_append_node(Deck *deck, Deck_node *deck_node); 

int deck_random_card_index(Deck *deck);

void deck_node_switch(Deck_node *n1, Deck_node *n2); 

void deck_shuffle(Deck *deck, int loops); 

Deck_node *deck_pop_head_node(Deck *deck);

void deck_populate(Deck *deck);

Deck *deck_draw_cards(Deck *deck, int num_cards, int remove); 

void printDeck(Deck *deck);

#endif

below is card.c

```
//card.c (lib)
#include
#include
#include "card.h"

Card card_create(int suit, int number, char symbol, int symbol_length) {
Card card = (Card)malloc(sizeof(Card) + (sizeof(char)*symbol_length));
if (card) {
card->suit = suit;
card->number = number;
card->symbol = symbol;
if (numberface = 0;

Here are some things that may help you improve your program.

Eliminate unused variables

This code declares a variable j in deck_draw_cards and has a parameter remove but then does nothing with either. Your compiler is smart enough to help you find this kind of problem if you know how to ask it to do so.

Ensure every control path returns a proper value

The deck_node_add_next function claims to return a Deck_node* but doesn't. That's an error that should be fixed.

Check the return value of malloc

If the program runs out of memory, a call to malloc can fail. The indication for this is that the call will return a NULL pointer. You should check for this and avoid dereferencing a NULL pointer (which typically causes a program crash). In some places the code properly checks, but in other places it does not.

Don't use system("clear")

There are two reasons not to use system("clear"). The first is that it is not portable to other operating systems which you may or may not care about now. The second is that it's a security hole, which you absolutely must care about. Specifically, if some program is defined and named clear, your program will execute that program instead of what you intend, and that other program could be anything. First, isolate this into a seperate function clearscreen() and then modify your code to call those functions instead of system. Then rewrite the contents of that functions to do what you want using C. For example, if your terminal supports ANSI Escape sequences, you could use this:

void clearscreen() {
    printf("\x1b[2J");
}

Initialize structures completely

There is a subtle bug in the program. The main routine calls deck_create() and then deck_populate() which looks normal. Then deck_populate() calls deck_append_node() which checks deck->tail and deck->head before using them. This also looks normal, but the problem is that neither head nor tail was actually initialized to any particular value. I'd recommend adding this line to deck_create:

deck->head = deck->tail = NULL;

There is also an identical issue with deck_node_create. A related but not quite identical issue exists within game(). The code mallocs the players_cards and then checks to see if they're NULL, but they haven't necessarily been initialized yet. I'd recommend simply creating all player hands immediately and then skipping the check within the loop.

Think of the user

The players in this game are referred to as "Player 1" and "Player 2" but the winner is declared to be either "Player 0" or "Player 1" which is not really right. Also, an Ace in blackjack can count as either 1 or 11, which the game currently doesn't understand, so if a player actually draws a blackjack, the program doesn't recognize it as a winning hand! (A Queen+Ace is actually tallied incorrectly as 13!) No blackjack game ever starts with only a single card dealt. Each player starts with two cards, so that's another aspect of the game that could be improved. It's also not at all obvious that when the game asks the user "Draw again?" that what it apparently expects is a number.

Don't leak memory

As currently written, the program is guaranteed to leak memory because both the player's hands and the main deck are never freed. Code to do that should be added, and deck_destroy() should be added to card.h.

Understand sizeof

Right now the code includes this line:

Card *card = (Card*)malloc(sizeof(Card) + (sizeof(char)*symbol_length));

First, sizeof(char) is defined to always be 1, so that's not necessary or useful. Instead, change that to

Card *card = malloc(sizeof(Card) + symbol_length);

Note that no cast is needed here. Another line is this one:

Card *card = card_create(j, i/4, symbol_map[i/4], sizeof(symbol_map[i/4])/sizeof(char));

First, again, sizeof(char) is defined to be one, so it should be omitted. More importantly, the sizeof(symbol_map[i/4]) is returning the size of the pointer and not the size of the string. The code currently only copies only the pointer, so, for example, every Ace in the deck points to the single string.

That's not a bad way to do it, but it means that the length parameter is completely useless. So I'd make the card names const and rewrite deck_populate and card_create, omitting the length parameter and understanding that all that is being stored in a Card is a pointer to an existing name. The revised card_create would look like this:

Card *card_create(int suit, int number, const char *symbol) {
    Card *card = malloc(sizeof(Card));
    if (card) {
        card->suit = suit;
        card->number = number;
        card->symbol = symbol;
        card->face = number>9;
    }
    return card;
}

And the revised deck_populate like this:

```
void deck_populate(Deck *deck) {
static const char *symbol_map[] = {"Ace", "2", "3", "4", "5", "6", "7", "8", "9"

StackExchange Code Review Q#156079, answer score: 5