0

I think my english is just to bad to understand the other articles about this. But anyway:

I just thought i could write a program (in C), that can store a set of cards. Not complicated, just store values and names of cards and print them out. I'm a beginner in C, and because i'm in the section "Strings in Structures" in my Book, i wanted to try out structures on my own. This is my Code so far:

#include <stdio.h>

struct card
{
    int value;
    char name[];
};

int main(void)
{
    const struct card heart[13] = { {2,"two"}, {3,"three"}, {4,"four"}, {5,"five"}, {6,"six"}, {7,"seven"}, {8,"eight"}, {9,"nine"}, {10,"ten"}, {11,"jack"}, {12,"queen"}, {13,"king"}, {14,"ace"} };
    int i;

    for (i = 0; i < 13; ++i)
    {
        printf("The card heart-%s has the value of %i", heart[i].name, heart[i].value);
    }

return 0;

}

I just wanted to test if it works, so i just wrote the heart-cards in the code. If i want to compile this file, my compiler (gcc/mingw) hits me with 26 errors. It says: "(near initialization of heart[0])" "non static initialization of a flexible array member" I don't really understand this. In the book, everything works as expected. I tried to rebuild the code in the book and changing the names, but it doesn't work. I think it's a problem with the strings, because if i use integers only, everything works. In already read in another post, that every string should be allocated manually, and there was a code example, but i don't know what all the lines should mean, and i want understand what my code does, so i don't copy + paste.

Could you explain me why this doesn't work? PS: I am writing currently in windows, so please don't use bash commands to explain or something like that. I am also german and my english is not the "yellow of the egg", try to explain without using complex 'sentence builds' (i hope you know what i mean :D) and unusual words.

Thanks for all help!

Improve this question
3
  • If your "compiler hits you with 26 errors", maybe you should try compiling something simpler first? Commented Apr 27, 2016 at 19:15
  • I suspect you need to allocate some chars for the card names. In struct card, try changing from char name[] to char name[16] Commented Apr 27, 2016 at 19:18
  • 1
    Or you can try const char *name. Commented Apr 27, 2016 at 19:26

2 Answers 2

Reset to default
3

You need to create some space for the name of each card. Easiest way to do this would be to change your struct card definition to something like:

struct card
{
  int value;
  char name[16];  // doesn't have to be 16, but make sure it's large enough to hold each card name plus a '\0' terminator
};
Improve this answer
Sign up to request clarification or add additional context in comments.

10 Comments

Assigning fixed storage for strings is often the root cause of buffer overrun exploits. A better answer would be to use pointers.
@JayM how would using pointers fix that problem? Every buffer has a finite amount of space, whether allocated statically or dynamically. It's up to the programmer to code defensively to ensure buffers aren't overrun. The method in which that space is allocated is irrelevant in terms of mitigating a buffer overrun problem.
@JayM Oh I see what you're saying in your answer. True, initializing with string literals would mitigate overrunning a fixed buffer during initialization. However, 1) 16 is clearly long enough for the names chosen by the OP (any decent compiler will warn of an overrun), 2) card names are not likely to change during runtime, and 3) even if they did, your method introduces the potential danger of modifying a string literal, which is UB. In both cases, it's up to the programmer to do the right thing.
All in all, I do agree using const char* would be the better option vs what I suggested here, based on the assumption that card names most likely won't change during runtime.
The compiler does now know of overruns at runtime, e.g. strcpy("Username: %s long name that's too long, so as to allow [sh su root]", card.name); I am precicely not overwriting a string liternal. Instead I suggest replacing valid pointers. A pointer is not a string literal, it's a pointer (to character array). That array can be as large as you have memory for, var or const. Safely replace with another that points to larger storage (either via malloc() or a pointer to different string literal) at runtime. e.g. card.name = "A very long string literal that can't be changed".
|
0

The prior answers suggest allocating a fixed length for your names. This has limitations and even dangers. It is always a good idea to avoid it all together.

e.g. You want to alter the name during the game, e.g. "Ace (Trump Card)" but that might be both too long even worse overwrite memory. (Many of the known vulnarabilities in code are caused by buffer overruns)

You are also building in a limitation; What if your game needs translating into another language?

By using pointers, you don't need to resort to either variable length structures or fixed string lengths.

You also add the ability to add API access functions that set data, allowing checks before it's written, preventing buffer overruns.

Instead of using character array (aka strings) you should use pointers in your structures. If you follow the link at the bottom I take this further and use pointers to the structures themselves.

As the pointer storage size never changes your names can be of any length and even altered later, perhaps as the game progresses.

Your card could look something like

typedef struct card
{
    int value;
    char * name;
}

Now the initial assignment can be done like this

card_t card_ace = {14, "Ace"};

And the values are not fixed (unless that is what you want, then you make them const).

card_ace.value = 200;
card_ace.name = "Trump card";

or an array of cards like this

card_t suit_hearts[] = {{2,"two"}, {3,"three"}, {4,"four"}, {5,"five"}, {6,"six"}, {7,"seven"}, {8,"eight"}, {9,"nine"}, {10,"ten"}, {11,"jack"}, {12,"queen"}, {13,"king"}, {14,"ace"}}

Even better make the whole thing using pointers

typedef card_t * cards_t;

cards_t mysuit = &(card_t){2,"two"}, &(card_t){3,"three"}, ...

Perhaps consider makeing the suit a structure.

typedef struct 
{
    char * name;
    card_t ** cards;
} suit_t;

typedef card_t * cards_t[];

suit_t mysuit = {
    .name = "Hearts",
    .cards = (cards_t){&(card_t){2,"two"}, &(card_t){3,"three"},....}
}

* For a fully working example of the latter, demonstrating using arrays of pointers to sidestep the limitations of variable length members of fixed arrays, see this gist on github

Improve this answer

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.