How do I write a function that generates an instance of a previously defined struct every time it's called? I'm sure since it's an easy problem no context is needed but here is what I have now.
#include <stdio.h>
#include <stdlib.h>
#include <time.h>
#include <string.h>
time_t t;
char names[][40] = {"Name1", "Name2", "Name3"};
struct fighter {
int type;
int strength;
int dexterity;
int resistance;
int weapon;
char name[];
};
struct fighter fighters[];
struct team {
struct fighter f1;
struct fighter f2;
struct fighter f3;
struct fighter f4;
struct fighter f5;
int wins;
int losses;
char name[];
};
struct fighter genf(){
struct fighter genfighter;
memcpy(genfighter.name, names[rand()], 40);
return genfighter;
};
int main(){
srand((unsigned) time(&t));
scanf("%s");
struct fighter f1 = genf();
scanf("%s");
}
And I get this on compilation with gcc.
In function 'genf':
note: the ABI of passing struct with a flexible array member has changed in GCC 4.4.
At top level:
warning: array 'fighters' assumed to have one element.
When running it I get a segmentation fault after one scanf()
CodePudding user response：
I tried out your code. You have a few bits going against you with the current code. First off, the "rand()" function can produce a very large value which causes an array element that does not exist in array "name" to be accessed. That's what is causing your segmentation fault. Also, if you happen to get past that bit, you will have a buffer overrun when trying to store a "name" array element as the "name" variable in your structure basically doesn't have any length. With that following is a tweaked version of your code.
#include <stdio.h>
#include <stdlib.h>
#include <time.h>
#include <string.h>
time_t t;
char names[][40] = {"Name1", "Name2", "Name3"};
struct fighter
{
int type;
int strength;
int dexterity;
int resistance;
int weapon;
char name[40]; /* Need to define a size or a buffer overflow will occur */
};
struct fighter fighters[];
struct team
{
struct fighter f1;
struct fighter f2;
struct fighter f3;
struct fighter f4;
struct fighter f5;
int wins;
int losses;
char name[40]; /* Need to define a size or a buffer overflow will occur */
};
struct fighter genf()
{
struct fighter genfighter;
printf("Value: %d\n", rand()); /* To illustrate what type of number is generated */
memcpy(genfighter.name, names[(rand() % 3)], 40);
return genfighter;
};
int main()
{
srand((unsigned) time(&t));
//scanf("%s");
struct fighter f1 = genf();
printf("Fighter name: %s\n", f1.name);
//scanf("%s");
return 0;
}
Note the the character arrays have been given a size that should accommodate the storage of a name. And a modulo value is derived from the "rand()" call to make sure that the index value will be valid. Following was a test run.
@Una:~/C_Programs/Console/Fighter/bin/Release$ ./Fighter
Value: 1876063575
Fighter name: Name3
I did not address the single element warning from the compiler. I will let you sort out the size of your structure array. But give that a try.
CodePudding user response：
You declared a structure with a flexible array data member name
struct fighter {
int type;
int strength;
int dexterity;
int resistance;
int weapon;
char name[];
};
You need to allocate dynamically memory for an object of the structure type and its flexible array name if you want to store in the array data as you are doing in the function genf
struct fighter genf(){
struct fighter genfighter;
memcpy(genfighter.name, names[rand()], 40);
return genfighter;
};
Otherwise the function invokes undefined behavior.
Also you may not have nested structures with flexible arrays as in this declaration
struct team {
struct fighter f1;
struct fighter f2;
struct fighter f3;
struct fighter f4;
struct fighter f5;
int wins;
int losses;
char name[];
};
As for this declaration in the file scope
struct fighter fighters[];
then it represents a tentative definition equivalent to
struct fighter fighters[1];
Pay attention to that you forgot to specify the second argument that corresponds to the format string "%s" in this call of scanf
scanf("%s");
So it invokes undefined behavior.
CodePudding user response：
To add to the other answers, scanf() requires a pointer to read into for each conversion specifier in the format string. You need:
char charstring[100]
scanf("%s", charstring)
However, this is still unsafe, because your user may enter more than 100 bytes, which would overflow the buffer. So this is better:
char charstring[100 1];
scan("0s", charstring);
charstring[100] = '\0';
I recommend turning -Wall on: your compiler may only warn you here, but it will likely cause a crash!
CodePudding user response：
"is there no way you can use an array for this?"
The arrays char name[] at the end of the struct fighter and struct team structures are of incomplete type. This is allowed as the last member of a struct, creating a flexible array member (or FAM). But, a struct with a member of incomplete type is itself of incomplete type. C does not allow creation of arrays with elements of incomplete type.
The easiest way to solve this problem is to choose a constant size for the name arrays which is large enough to accommodate expected names and to use regular array members of constant size to store the names. Before initializing these arrays the lengths of names should be validated and names which are too long handled.
#include <stdio.h>
#include <string.h>
#define NAME_SZ 20 // Small size for testing purposes
#define MAX_FIGHTERS 100
struct fighter {
int type;
int strength;
int dexterity;
int resistance;
int weapon;
char name[NAME_SZ];
};
struct fighter fighters[MAX_FIGHTERS];
struct fighter genf(const char *name) {
// Ensure that all members are zero initialized.
struct fighter genfighter = { 0 };
size_t name_sz = strlen(name) 1; // add 1 for `\0` terminator
// Can only copy at most `NAME_SZ` characters.
if (name_sz > NAME_SZ) {
name_sz = NAME_SZ;
}
memcpy(genfighter.name, name, name_sz);
// Add terminating `\0` if needed.
if (name_sz == NAME_SZ) {
genfighter.name[NAME_SZ-1] = '\0';
}
return genfighter;
}
int main(void) {
char names [][NAME_SZ] = {
"Apocalypse Alan",
"Burly Barry",
"Cederick the Clever",
};
size_t names_num = sizeof names / sizeof *names;
size_t fighters_num = 0;
// Populate `fighters` array.
for (size_t i = 0; i < names_num; i ) {
fighters[i] = genf(names[i]);
fighters_num; // Increment number of fighters.
}
// Let's try to add a couple of names that are too long.
fighters[fighters_num ] = genf("Cederick the Cleaver");
fighters[fighters_num ] = genf("Derrick of Doubtful Kindness");
// Show names of fighters in `fighters` array.
for (size_t i = 0; i < fighters_num; i ) {
printf("Fighter #%zu: %s\n", i 1, fighters[i].name);
}
return 0;
}
Compiling and running the above program yields:
$ gcc -std=c17 -Wall -Wextra -pedantic fighterstructs.c
$ ./a.out
Fighter #1: Apocalypse Alan
Fighter #2: Burly Barry
Fighter #3: Cederick the Clever
Fighter #4: Cederick the Cleave
Fighter #5: Derrick of Doubtful
Note that the last two names that were added, "Cederick the Cleaver" and "Derrick of Doubtful Kindness", are too long to be stored in a names array (which at present holds 20 chars, or a string of maximum length 19). But the genf function checked to see if the name would fit in a names array before applying memcpy, only copied the initial portion of the name that would fit, and made sure that the result was null-terminated. "Cederick the Cleaver" is only one char too long; the array holding this string must be 21 chars long. If this was copied without checking length then code would write past the end of the name array, causing undefined behavior. Copying only the first 20 chars avoids a buffer overflow, but the resulting array is not null-terminated. An additional check must be made to ensure that a null-terminated array is created.
Finally, make sure that you always compile code with a generous number of warnings enabled. A good start is gcc -std=c17 -Wall -Wextra -pedantic.