CODEWITHSUNDEEP
cfilepointersstructsegmentation-fault
Pepe Fernandez
Pepe Fernandez

Reputation: 145

Filling struct with data in file

I'm trying to fill a struct with the data of a file.

The file is separated with double dot like this:

string1:15

When I read the file and fill the fields of the struct with data I get a Segmentation Fault.

Here the struct file character.h:

#ifndef CHARACTER_H_
#define CHARACTER_H_

typedef struct Character *Personaje;

struct Character{
        char name[20];
        int lvl;
};


extern void saveCharacter(char *name, int lvl);
extern Personaje *getCharacter();
extern char *toString(Personaje *pj);
#endif

and here the function in source file character.c:

Personaje *getCharacter(){
        FILE *fp;
        Personaje *salida = (Personaje*) malloc(sizeof(Personaje));
        fp = fopen("kprct", "rb");

        fscanf(fp, "%[A-Za-z]:%d", (*salida)->name, &((*salida)->lvl));
        printf("Linea: %s : %d\n", (*salida)->name, (*salida)->lvl);

        fclose(fp);
        return salida;

}

How can I fill the struct with the file data?

Upvotes: 0

Views: 185

Answers (2)

too honest for this site
too honest for this site

Reputation: 12263

Hint: do not typecast a pointer to your struct, but the stuct itself. This is much less confusing and will avoid exactly the problem you are facing here: only allocate space for the pointer, not the struct.

Also you are defining a Character **:

Personaje *salida = (Personaje*) malloc(sizeof(Personaje));

So: with the given type definitions:

Personaje salida = malloc(sizeof(*salida));

Note: using *salida in sizeof makes this term independent from the type of salida.

The rest of your code is also broken, as your functions also have one * too much whereever Personaje is involved.

A proper definition would be:

typedef struct {
        char name[20];
        int lvl;
} Personaje;     // or name the type just Character

Then, for the prototypes, use Personaje * as you already have. The malloc-line would be:

Personaje *salida = malloc(sizeof(*salida));

(Note: I did not have to change the sizeof()-argument.)

Additional issues:

  • Always check the results of system functions, as they might report an error. malloc might return NULL, the file functions might report an error, too.
  • Always restrict the number of chars fscanf reads into a char array! As it is now, it is an invitation to buffer overflow, aka undefined behaviour.
  • Do not cast void * as used by malloc, free, etc.

Upvotes: 1

Sourav Ghosh
Sourav Ghosh

Reputation: 134356

I think, the major problem here is

typedef struct Character *Personaje;

this way, you're making Personaje as a pointer already, and in the code, you're writing

  Personaje *salida

which creates a pointer to pointer. This is not required, an neither what you wanted, I think. I suggest, change your typedef to

typedef struct Character Personaje;

and all the access to the variable in fscanf() and printf().

Also,

  1. Please do not cast the return value of malloc() and family in C.
  2. Check for the return value of fopen() and fscanf() to ensure success.

Upvotes: 2

Related Questions