Problems with structures with pointers to arrays of other structures in C

Question

Hi there!

I am attempting to tackle college worksheet on C programming (no marking for it, just to improve our learning). What we're meant to do is get a few details about shipping docks. I decided to use structures for this.

My code is below, what I need help with is to print out the information (to see if its working) of whats at the location of the shipyards .run.

Everything compiles and according to the debugger shipyard1.run and shipyard2.run point to different locations, but I can not see the values.

int main(int argc, char** argv)
{
    typedef struct dockInfo
    {
        int dockCode;
        int dockLength;
    }dckDetails;

    typdef struct shipyard
    {
        char dockName[20];

        /* however big this number is thats how many dockInfo structs are needed.*/

        int numOfDocks;             
        dckDetails *run; //points to the array of dockInfo structs

    };

    struct dockInfo *arrayD; // the array to hold all the dockInfo structs
    struct dockInfo tempo; // the temporary dockInfo struct to take in the details
    struct shipyard shipyard1;
    struct shipyard shipyard2;

    /**
    * the variables for shipyard1 and shipyard2 are then assigned
    **/

    int i;
    for (i=0;i<shipyard1.numOfDocks;i++)
    {

    arrayD=calloc(shipyard1.numOfDocks,100); // allocate a new bit of memory for arrayD
    tempo.dockCode=45*i;
    tempo.dockLength=668*i;
    arrayD[i]=tempo; //the element of arrayD becomes tempo.

    }
    shipyard1.run=arrayD; //make shipyard1.run point to the location of arrayD.

    for (i=0;i<shipyard2.numOfDocks;i++)
    {

    arrayD=calloc(shipyard2.numOfDocks,100); // allocate a new bit of memory for arrayD
    tempo.dockCode=1234*i;
    tempo.dockLength=1200*i;
    arrayD[i]=tempo; //the element of arrayD becomes tempo.

    }
    shipyard2.run=arrayD; //make shipyard2.run point to the new location of arrayD.

    int elementTest1; // need element1test to be shipyard1.run[0].dockLength;
    int elementTest2; // need element2test to be shipyard2.run[1].dockCode;

return (EXIT_SUCCESS);
}

It should be noted that I have left a lot of code out because I have yet to write it. I have used static examples for the moment (shipyard1 and shipyard2) but in the future I am going to implment a 'load info from file' feature.

Any help would be greatly appreciated and please excuse my English if it's poor, English is not my first language.

Answer 1

You have calloc() inside a for loop twice. Both times you're losing the address returned.

for () {
    addr = calloc();
    addr[i] = ...
}

the second time through the loop, the addr you got on the first time is gone (you got yourself a memory leak), the value you saved there is gone too.

Move the calloc() outside the loop ... and remember to free() the memory when you no longer need it

addr = calloc();
for () {
    addr[i] = ...
}
free(addr);

Answer 2

Some feedback:

• The memory allocation parts with calloc should occur outside the loop. Now you allocate it, and then loose track of it in the next iteration because new memory is allocated and assigned.
• memory you allocate should be freed somewhere with free
• shipyard1.numOfDocks (same for shipyard2) is unitialized when you use it, it may be a random number (which means you have an undefined number of loop iterations, and allocate an undefined amount of memory).

Good luck!

Answer 3

Others have made some very good points, and you should fix your code according to them. So far, no one seems to have seen that the call to calloc() is wrong. Instead of:

arrayD=calloc(shipyard1.numOfDocks,100);

it should be:

arrayD = calloc(shipyard1.numOfDocks, sizeof *arrayD);

You want shipyard1.numOfDocks objects, each of size equal to sizeof *arrayD.

In fact, as mentioned below, you don't need to set the memory allocated to all-zeros, so you can replace calloc() by malloc():

arrayD = malloc(shipyard1.numOfDocks * sizeof *arrayD);

(Be sure to #include <stdlib.h>, whether you call calloc() or malloc().)

I have some minor comments about style:

1. you don't need the typedef. You can write struct dockInfo instead of dckDetails. If you do keep the typedef, you should be consistent, and use the typedef name everywhere. You use struct dockInfo most of the time, and then use dckDetails once. Your usage suggests that you probably weren't comfortable declaring a pointer to the struct. However, struct dockInfo *run is a completely valid declaration.
2. you don't need the tempo object. You can instead do: arrayD[i].dockCode = 45*i; arrayD[i].dockLength = 668*i;
3. Unless you're running C99, you can't declare variables after statements in a block. So you should move the declarations for elementTest1 and elementTest2 to the top of main(), with other declarations.
4. return is a statement, not a function, so the parentheses are not needed.
5. Since you overwrite the memory allocated immediately, and don't need it to be zero, you can replace calloc() call by a suitable call to malloc().

As I said, these are minor comments. Your main problems lie with the wrong use of calloc, etc.