CODEWITHSUNDEEP
cpointersstructinitializationallocation
OneZero
OneZero

Reputation: 11904

Initializing struct in C

OK, this is the definition of the struct:

typedef struct {
   int first;
   int last;
   int count;
   char * Array [50];
} queue;

and I use another function to initialize it

void initialize(queue * ptr){
   ptr=malloc(sizeof(queue));
   ptr->first=0;
   ptr->last=0;
   ptr->count=0;
}

Then I use printf to print out first, last and count. All three should be zero. However, what I actually get is, count is 0 as I expected, but first&last are two very large strange numbers and they change every time I run the program. Can anybody tell me what's wrong here? Thank you.

Upvotes: 1

Views: 1577

Answers (3)

Heath Hunnicutt
Heath Hunnicutt

Reputation: 19457

Here is the smallest alteration to your program which should correct your problem:

void initialize(queue * * pptr) {
    queue * ptr;
    ptr=malloc(sizeof(queue));
    if (ptr) {
        ptr->first=0;
        ptr->last=0;
        ptr->count=0;
    }
    /* The assignment on the next line assigns the newly allocated pointer
       into memory which the caller can access -- because the caller gave
       us the address of (i.e. pointer to) such memory in the parameter
       pptr. */
    *pptr = ptr;
}

The most important change is to pass a queue ** to your initialize function -- otherwise you are changing a copy of the queue * supplied as the actual parameter when you call initialize(). By passing a pointer to the pointer, you can access the original variable which stores the pointer in your caller.

I couldn't resist and also added a check for NULL returned from malloc(). That doesn't address your problem, but I couldn't bring myself to post code that didn't do it.

Upvotes: 0

justin
justin

Reputation: 104698

Given:

void initialize(queue * ptr);

Pass it like this:

queue q; // caller allocates a queue
initialize(&q);
// now q is initialized

Also, it's allocated by the caller -- don't malloc it.

// bad
void initialize_bad(queue * ptr){
   ptr=malloc(sizeof(queue)); << nope. already created by the caller. this is a leak
   ptr->first=0;
   ptr->last=0;
   ptr->count=0;
}

// good
void initialize_good(queue * ptr){
   ptr->first=0;
   ptr->last=0;
   ptr->count=0;
   // ptr->Array= ???;
}

If you prefer to malloc it, then consider returning a new allocation by using this approach:

queue* NewQueue() {
   // calloc actually works in this case:
   queue* ptr = (queue*)calloc(1, sizeof(queue));
   // init here
   return ptr;
}

Ultimately, what is 'wrong' is that your implementation passes a pointer by value, immediately reassigns the pointer to a new malloc'ed allocation, initializes the malloc'ed region as desired, without ever modifying the argument, and introducing a leak.

Upvotes: 3

Mark Byers
Mark Byers

Reputation: 837916

You are passing your pointer by value. The function changes a copy of the argument it receives, but the caller's pointer is not modified and is probably unintialized.

You need to change your function to take a queue** and pass the address of the pointer you want to initialize.

Alternatively you could return a pointer instead of passing it in as an argument. This is a simpler approach.

Upvotes: 7

Related Questions