Initialize the arrays inside an array of char* using a loop

Question

I need to create an array of strings, each representing a card of the Spanish deck:

#define __USE_MINGW_ANSI_STDIO 1
#include <stdlib.h>
#include <stdio.h>
#include <string.h>

int main(void)
{
    char *type[4]= {"copas", "basto", "espada", "oro"};
    char *number[10]= {"Uno", "Dos", "Tres", "Cuatro", "Cinco", "Seis", "Siete", "Diez", "Once", "Doce"};
    char *deck[40];
    int deckIndex= 0;
    for (int i = 0; i < 4; i++)
    {
        for (int j = 0; j < 10; j++) {
          char card[100] = "";
          strcat(card, number[j]);
          strcat(card, " de ");
          strcat(card, type[i]);
          strcat(card, "\n");
          deck[deckIndex]= card;
          deckIndex++;
        }
    }
    for (int i = 0; i < 40; i++)
    {
        printf("%s\n", deck[i]);
    }
    return 0;
}

However, all entries of deck[] point to the same string. As a result, "Doce de oro" is printed 40 times. I don't understand why this happens, but I've theorized it's because card[] is being reinitialized in the same memory direction, and overrides what was already written there in the previous iteration. If I'm right, I would have to declare every array separately, but I have no idea how to do that without writing 40 different arrays.

Tldr: ¿Why do all entries of deck[] point to the same location? ¿How do I fix it?

(Btw suggestions for a better title are appreciated)

"all entries of deck[] point to the same string". It's worse than that. card only exists for one iteration of the loop. It becomes invalid after the end of each iteration. So accessing it outside the iteration and outside the loop results in Undefined Behaviour. Use strdup to make a dynamic copy. Don't forget to free the memory. — kaylum
– kaylum, Commented Oct 19, 2021 at 22:50
FYI you can use sprintf to construct your card string in one simple statement rather than making a sequence of strcat calls. Also, you probably do not want to append newline to the card. — jarmod
– jarmod, Commented Oct 19, 2021 at 22:54

Abdul Mannan Syed · Accepted Answer · 2021-10-19 22:56:53Z

2

In C, memory on the stack is allocated in terms of Scopes. So yes, your theory is right. You are rewriting on the same location.

To fix your program, there are two possible solutions I can think of.

You can use Multidimensional Arrays.
Or you can allocate memory in heap using malloc (but make sure to free it once you are done with it)

answered Oct 19, 2021 at 22:56

Abdul Mannan Syed

513 bronze badges

Sign up to request clarification or add additional context in comments.

Comments

Adrian Mole · Accepted Answer · 2021-10-20 01:01:30Z

As pointed out in the comments, in the deck[deckIndex]= card; line, you are assigning the same pointer¹ to each of your deck elements – and, worse, a pointer to a variable (the card array) that is no longer valid when the initial nested for loop goes out of scope.

To fix this, you can make copies of the card string, using the strdup function, and assign the addresses of those copies to the deck elements. Further, as also mentioned in the comments, you can simplify the construction of the card string using a single call to sprintf, rather than using multiple strcat calls.

Here's how you might do that:

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

int main(void)
{
    char* type[4] = { "copas", "basto", "espada", "oro" };
    char* number[10] = { "Uno", "Dos", "Tres", "Cuatro", "Cinco", "Seis", "Siete", "Diez", "Once", "Doce" };
    char* deck[40];
    int deckIndex = 0;
    for (int i = 0; i < 4; i++) {
        for (int j = 0; j < 10; j++) {
            char card[100] = "";
            sprintf(card, "%s de %s", number[j], type[i]);
            deck[deckIndex] = strdup(card);
            deckIndex++;
        }
    }
    for (int i = 0; i < 40; i++) {
        printf("%s\n", deck[i]);
    }

    // When you're done, be sure to free the allocated memory:
    for (int i = 0; i < 40; i++) {
        free(deck[i]);
    }
    return 0;
}

If your compiler does not support the strdup function (most do, and it is part of the ISO C Standard from C23), writing your own is very simple:

char* strdup(const char *src)
{
    char* result = malloc(strlen(src) + 1); // Add one to make room for the nul terminator
    if (result) strcpy(result, src);
    return result;
}

¹ Well, formally, a new card array is born on each iteration of the inner for loop, but it would be a very inefficient compiler that chose to do that, rather than simply re-using the same memory – which is clearly what is happening in your case.

Collectives™ on Stack Overflow

Initialize the arrays inside an array of char* using a loop

2 Answers 2

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related