error creating dynamic memory for a structure. C

[MisterWhite]

i have this struct:

typedef struct
{
    int score;
    int time;
    char *name;//will have space for 12 chars
}HighScore;

in main i have this:

HighScore *highScore=NULL;//will have 10 records
menuScreen(&highScore,&highScoreRecords);//scoreRecords is just number, not an issue

now functions:

int menuScreen(HighScore **highScore,int *records)// records all good
{
.....
	getHighScore(highScore,records);// passing correctly?
.....
}

int getHighScore(HighScore **highScore,int *records)
{
....
	int nr=0;
	highScore=malloc(sizeof(HighScore)*10);//to have memory for 10 records
	while(!feof(highScoreTable))
	{
	  //segmentation fault in this allocation
      highScore[nr]->name=malloc(sizeof(char)*12);// so that name would have memory for 12 chars
	  //highScoreTable is a FILE*
      fscanf(highScoreTable,"%d %d %s\n",&(highScore[nr]->score),&(highScore[nr]->time),highScore[nr]->name);//am i reading this correcly?
      nr++;
	}
....
}

I had this done with arrays but now i have to do them with dynamic memory and i'm not sure what 'i'm doing wrong

[Unimportant]

First of all, you can create a array of 12 char's straight in the struct definition, that saves you a malloc, a free, and a potential leak:

typedef struct
{
    int score;
    int time;
    char name[12];
}HighScore;

 

Secondly, you pass a pointer to the highScore pointer to your function, in order to use the highScore pointer itself you must first dereference the pointer to the highScore pointer:

int getHighScore(HighScore **highScore,int *records)
{
....
	int nr=0;
  	//* dereferences the pointer to HighScore pointer, so you have acces to highScore pointer
	*highScore=malloc(sizeof(HighScore)*10);//to have memory for 10 records
	while(!feof(highScoreTable))
	{
      		//Following line is no longer needed, because of the change we made to the struct...
	  	//highScore[nr]->name=malloc(sizeof(char)*12);// so that name would have memory for 12 chars
      
	  	//highScoreTable is a FILE*
      		fscanf(highScoreTable,"%d %d %s\n",&(*highScore)[nr].score ,&(*highScore)[nr].time, (*highScore)[nr].name);
      		nr++;
	}
....
}

 

The fscanf line is complex so let's go over it in more detail:

//highScore is a pointer to a HighScore pointer and we want to get pointers to the members to pass to fscanf...
//First , we need to dereference the pointer to pointer to get access to the pointer:

	*highScore

//Then, we need to index the array:

	(*highScore)[nr]

//Because [] also dereferences the pointer we need to access the members using . , not ->
	
	(*highScore)[nr].score

//But we need the address of this member, not the member itself:

	&(*highScore)[nr].score

//But, member name is a array, array names decay into a pointer so we do NOT need to take it's address:

	(*highScore)[nr].name  //<-- note the missing &

And now have a drink to soothe your brain

[MisterWhite]

44 minutes ago, Unimportant said:

snip

Thank you for your reply. But the task is to use dynamic memory instead of arrays so that's what i have to do.

[Unimportant]

But you are using dynamic memory for the entire struct (malloc), there is no point in allocating members separately unless they vary in length.

 

Anyway, everything i said still applies, if you really need to allocate the name member dynamically aswell:

int getHighScore(HighScore **highScore,int *records)
{
....
	int nr=0;
  	//* dereferences the pointer to HighScore pointer, so you have acces to highScore pointer
	*highScore=malloc(sizeof(HighScore)*10);//to have memory for 10 records
	while(!feof(highScoreTable))
	{ 	
      		(*highScore)[nr].name=malloc(sizeof(char)*12);// so that name would have memory for 12 chars
      
	  	//highScoreTable is a FILE*
      		fscanf(highScoreTable,"%d %d %s\n",&(*highScore)[nr].score ,&(*highScore)[nr].time, (*highScore)[nr].name);
      		nr++;
	}
....
}

 

[BobKelso]

Hi,
I noticed some other errors...

You only allocate Memory for 10 scores but you don't exit the loop after 10 Iterations.

And you asking for a buffer overflow in your fscanf since you don't restrict the amount characters that you read in for the name. You can provide scanf functions with a field width. "%11s" would do the trick for you.

 

To highlight the buffer overflow I created the following example for you:

int main(int argc, char **argv)
{
    const char lString[] = "some_really_long_string";
    char random1[8];
    char toShort[4];
    char random2[8];

    strncpy(random1, "random", sizeof(random1));
    strncpy(random2, "random", sizeof(random2));

    sscanf(lString, "%s", toShort);
    printf("toShort: %s\n", toShort);
    printf("random1: %s random2: %s\n", random1, random2);
    return 0;
}

And here the output of my Application:

./test 
toShort: some_really_long_string
random1: random random2: ing

As you can see random2 two was overridden by the ing of  "some_really_long_string" and to short appears to be holding way more than it should. 

 

As a Developer never trust that input is formatted in a specified way! Always make sure you don't read more data in that you can store  

 

Best Regards

Timo