CODEWITHSUNDEEP
cmallocstructure
ovnia
ovnia

Reputation: 2500

C structures, memory alloc and structures

So, the task is to read file and push data to structure. the data file is:

babe 12 red 12
deas 12 blue 12
dsa 12 red 512
bxvx 15 blue 52
reed 18 black 15

while code is something like that

struct shoes {
    char name[8];
    int size;
    char color[8];
    int price;       
};
//open file
shoes *item=(struct shoes*)malloc(sizeof(struct shoes));
for (i=0; !feof(file); i++) {
    item=(struct shoes*)realloc(item,sizeof(struct shoes)+i*sizeof(struct shoes));
    fscanf(file,"%s %i %s %i\n",(item+i)->name,(item+i)->size,(item+i)->color,(item+i)->price);
}

but the program crashes every time. dbg says: No symbol "item" in current context. where is error?

Upvotes: 1

Views: 219

Answers (4)

Jim Balter
Jim Balter

Reputation: 16406

You code has some bugs: 1) You need a typedef for shoes unless you're compiling with a C++ compiler ... but then you should have tagged this C++. 2) feof doesn't return false until an attempt has actually been made to read beyond the end of file, so your code makes the array of shoes 1 too large. 3) You're passing ints to fscanf instead their addresses.

If compiling as C code, not C++ code, the casts on malloc and realloc aren't necessary and are advised against. And there are other stylistic issues that make you code harder to comprehend than it could be. Try this:

typedef struct {
    char name[8];
    int size;
    char color[8];
    int price;       
} Shoe;

// [open file]

Shoe* shoes = NULL; // list of Shoes
Shoe shoe; // temp for reading

for (i = 0; fscanf(file,"%s %i %s %i\n", shoe.name, &shoe.size, shoe.color, &shoe.price) == 4; i++)
{
    shoes = realloc(shoes, (i+1) * sizeof *shoes);
    if (!shoes) ReportOutOfMemoryAndDie();
    shoes[i] = shoe;
}

Upvotes: 3

Mppl
Mppl

Reputation: 961

There are two issues with your code:

1st. The struct:

you could define your struct in two ways, the way you did:

struct shoe{    
 char name[8];
 int size;
 char color[8];
 int price;  
};

and in this case you should refer to a pointer to it as:

struct shoe *item;

the other (perhaps more convenient?) way using a typedef along with the defenition:

typedef struct {    
 char name[8];
 int size;
 char color[8];
 int price;  
} shoe;

and in this case you should refer to a pointer to it as:

shoe *item;

Therefore the code you posted isn't supposed to compile.

2nd one:

fscanf should be given pointers to integers/chars in the case you showed. You've correctly passed the char pointer(since you passed the name of a char array which is actually a char pointer to the first element), but you have passed some integers and fscanf needs pointer to integers it is supposed to fill, so your code should be:

fscanf(file,"%s %i %s %i\n",(item+i)->name,&((item+i)->size),(item+i)->color,&((item+i)->price));

Upvotes: 1

Mike
Mike

Reputation: 49393

Are you sure the debugger says that? I'm surprised it even compiled...

You wanted:

struct shoes *item

If you don't proved a typedef of your structure, you have to explicitly say "struct" every time you reference it.

Second note:

item=(struct shoes*)realloc(item...

Don't assign the same pointer from realloc() that you pass into it. If the reallocation fails it will return NULL and that could be killing you. You should make sure you're checking the results of both the initial malloc() and the realloc()

Third point:

You need to be passing the address of the int's to the fscanf().

fscanf(file,"%s %i %s %i\n",(item+i)->name,&((item+i)->size),(item+i)->color,&((item+i)->price));

Upvotes: 2

Nik Bougalis
Nik Bougalis

Reputation: 10613

The problem is that you are not passing pointers to the integers you want to read using fscanf, you are passing the integers themselves.

fscanf treats them as pointers. And where do they point? Who knows - the integers were uninitialized, so they could point ANYWHERE. Therefore, CRASH.

Fix thusly:

fscanf(file,"%s %i %s %i\n",
    (item+i)->name,
    &((item+i)->size),
    (item+i)->color,
    &((item+i)->price));

Note that you don't need the same for name and color because arrays degenerate to pointers, so you're passing the right thing already.

And please, consider ditching the item+i notation; item[i] is so much clearer and easier to understand when casually reading the code:

fscanf("%s %i %s %i\n", 
    item[i].name, 
    &item[i].size, 
    item[i].color,
    &item[i].price);

Upvotes: 3

Related Questions