problem printing out structure after loading

hi,
I am reading from a file which has 6 records(for now) and loading them into a structure. but when i try to print it back, it only prints the last record 6 times. it prints all 6 records right after loading it. but printing the whole structure later fails. i know theres a prob but am unable to figure out. any help will be appreciated. thanx in advance. here are some snippets of my code...

2 structures...

Code:

---

typedef struct Cars{
        int id;
        char make[SIZE];
        char model[SIZE];
        char color[SIZE];
        int year;
        long price;
        CAR_TYPE type;
}Car;

typedef struct CarInventory{
        Car **pList;
        int maxSize;
        int curSize;
}CarInv;

---

memory allocation
cInv.pList = (Car **)calloc(cInv.maxSize, sizeof(Car *));(in main)

Code:

---


BOOL bReallocateMemory(CarInv *cInv, Car *aCar)
{
        if(cInv->maxSize <= cInv->curSize )
        {
                cInv->maxSize = (cInv->maxSize >= 1) ? cInv->maxSize * 2 : 1;
                printf("Reallocating array size to %d", cInv->maxSize);
                cInv->pList = (Car **)realloc(cInv->pList, cInv->maxSize * sizeof(Car *));

                if(cInv->pList == NULL)
                {
                        printf("realloc failed\n");
                        return FALSE;
                }
        }
        cInv->pList[cInv->curSize] = (Car *)malloc(sizeof(Car));
        if(cInv->pList[cInv->curSize] == NULL)
        {
                printf("malloc failed\n");
                return FALSE;
        }
        cInv->pList[cInv->curSize] = aCar;
        printf(" id is cInv->pList[%d]->%d", cInv->curSize, cInv->pList[cInv->curSize]->id);
        printf(" make is cInv->pList[%d]->%s", cInv->curSize, cInv->pList[cInv->curSize]->make);
        (cInv->curSize)++;

        return TRUE;
}

---

printing in another function

Code:

---

for(i = 0; i < cInv->curSize; i++)
        {
                printf("\n%2d %16s %16s %16s %6d %8ld %6s",
                                cInv->pList[i]->id,
                                cInv->pList[i]->make,
                                cInv->pList[i]->model,
                                cInv->pList[i]->color,
                                cInv->pList[i]->year,
                                cInv->pList[i]->price,
                                carType[cInv->pList[i]->type]
                                );
        }

---

what if you split the print call into seperate print calls? any effect? sorry i don't deal with c much, mostly c++

i tried tht too.......but doesn't work..... i don't know wht else to do. I have been trying to figure it out...but no such luck. thanks anyway :-)

First up, I see you're using calloc() to create memory for pointers. In which case you need to read this which basically says don't do it.

Quote:

---

...does not therefore guarantee useful null pointer values...

---

>>cInv.pList = (Car **)calloc(cInv.maxSize, sizeof(Car *));
What is cInv.maxSize set to at this point (printf() it out to make sure its what you expect.

>>cInv->pList[cInv->curSize] = (Car *)malloc(sizeof(Car));
Same question again, this time for curSize. Also, are you sure it's within the bounds of pList.

>>cInv->pList = (Car **)realloc(cInv->pList, cInv->maxSize * sizeof(Car *));
This is bad practice. I realloc() fails, you've lost your pointer to the memory you had originally. A better way is to store the return in a temp variable, and if that's not NULL, assign it to pList.

i printed out maxSize & it was 0. so i did a malloc. is this right?

Code:

---

cInv.pList = (Car **)malloc(sizeof(Car *));
if(cInv.pList == NULL)
{
        printf("malloc failed\n");
        return 3;
}

---

but still the same problem persists.

initially curSize is 0, but i keep incrementing it when user wants to add a record

pList is a pointer to a pointer to a structure & i keep mallocing the array as well as each element in the array to store a record. theres no limit to how many records i can add

as to the statement below,

Code:

---

cInv->pList = (Car **)realloc(cInv->pList, cInv->maxSize * sizeof(Car *));

---

how can i declare a temporary var as pList is of type Car **?

Sorry, I should have said this in my first post..... why don't you scrap the array of pointers and just make the thing a link list?

i still havn't leant about linked lists.......sorry!

i vote for linked lists as well....unless this is homework and you haven't gone over them in class yet :)

Quote:

---

Originally posted by pari
i still havn't leant about linked lists.......sorry!

---

But they're not hard.... especially compared with what you're already doing.

>>cInv->pList[cInv->curSize] = aCar;
aCar is passed into the function, can you show the section of code that does that. Is aCar a pointer to malloc()'d memory?

yes u r right. i don't even know anything abt linked list. gotta do it this way.... & i can add records too now. it writes to file, loads to memory...... but the only thing which it isn't doing is printing from memory......i donno whts wrong......... sorry i shld say printing properly, coz it prints the last record as many times as there are records :-(

no. aCar is just an object..... Car aCar. do u want me to post the whole code?

printing? Where is your code to print all of the data? The only bit I see is within the bReallocateMemory() function, but that will print each one as it's added. I presume you have another function to print all the data, yes?

hmmm

sounds like hammer spotted a bug ;)

yes i do & this is it....

Code:

---

void vPrintList(const CarInv *cInv)
{
        int i = 0;
        int index = 0;
        char carType[6][10] = {"", "SEDAN", "COUPE", "SPORT", "SUV", "TRUCK"};

        for(i = 0; i < cInv->curSize; i++)
        {
                printf("\n%2d %16s %16s %16s %6d %8ld %6s",
                                cInv->pList[i]->id,
                                cInv->pList[i]->make,
                                cInv->pList[i]->model,
                                cInv->pList[i]->color,
                                cInv->pList[i]->year,
                                cInv->pList[i]->price,
                                carType[cInv->pList[i]->type]
                                );
        }
        return;
}

---

>>no. aCar is just an object..... Car aCar.
Hey presto, we have a problem. :)

Car aCar; <-- This will give you ONE car in memory.

If you pass the address of this car to the bReallocateMemory() function, and get it assigned to the array, each element in that array will point to the same aCar in memory, because there is only one. If you re-populate that aCar with new data, you'll only ever see the data from last one you entered.

Now to fix this, you need to malloc() memory to hold the Car data for each car you want, but you must also remember to free() it later as well. Things are getting much more complex now..... which is why a link list would be better. But hey, its up to you... good luck :) Keep asking if you don't understand, btw.

ohmygod thts abs great! u r a genius!
but i have a Q. can i not pass a copy of aCar instead of the address? will it not then be solved?

Passing structs around is not good practice, as they tend to be rather big (or can get to be so), and cause problems, so stick with passing addresses.

>>cInv->pList[cInv->curSize] = (Car *)malloc(sizeof(Car));
Here you malloc()'d memory to hold the structure. All you need do now is put the structure in it.
>>cInv->pList[cInv->curSize] = aCar;
Here you've goofed, because you're overwritten your pointer to your malloc()'d memory with another pointer. Now you can't free the memory you malloc()'d.

So, to fix it:
Change:
>>cInv->pList[cInv->curSize] = aCar;
To :
>>*(cInv->pList[cInv->curSize]) = *aCar;
I think the syntax is correct, it's late and I could be wrong!

Basically you can copy structs by direct assignment.

it works!!! thank u so much. i was trying to figure this out for the last 6 hrs. shows how dumb i am. :-) thanx again!