2

Suppose I have the following C code:

#include <stdio.h>
#include <stdlib.h>

#define NUM_PEOPLE 24

typedef struct {
    char **name;
    int age;
} person_t;

void get_person_info(person_t *person);

int main(int argc, char **argv) {
    for (int i = 0; i < NUM_PEOPLE; i++) {
        person_t new_person;
        get_person_info(&new_person);
    }

    return 0;
}

where get_person_info() just fills out the person_t struct to which a pointer is passed in. Is it necessary to malloc() memory for new_person within main()? That is, should the line

person_t new_person;

instead be

person_t *new_person = (person_t *) malloc(sizeof(person_t));

and then change get_person_info() to accept a person_t ** instead of a person_t *?

Sorry if this question is confusing -- I'm not sure whether or not this is a case where it is necessary to reserve memory, given that a pointer to that memory is passed into get_person_info() to avoid causing a segmentation fault.

legends2k
  • 31,634
  • 25
  • 118
  • 222
  • 1
    Both are right. The first one allocates memory on the stack while the second one allocates memory on the heap. – Spikatrix Sep 11 '15 at 10:41
  • Hmm...but if new_person will not be used in another function and only used in calling `get_person_info()` and within `main()`, is it safe to just store it on the stack and not bother `malloc()`ing memory for it? – Ryan Davidson Sep 11 '15 at 10:43
  • 4
    Both work. To determine which is better, need to see how `new_person` is later used in code. – chux - Reinstate Monica Sep 11 '15 at 10:44
  • Objects on the Stack are destroyed when the Block ends. If you need full control over the object's lifetime, you should allocate memory on the heap for it. – Fang Sep 11 '15 at 10:46
  • 2
    Note that, as the question is tagged with C and not C++, that casting the return value of `malloc` is redundant. – Johann Gerell Sep 11 '15 at 11:00

4 Answers4

2

Both are correct, it depends on where you want to use the person_info. Allocating on the stack :

 for (int i = 0; i < NUM_PEOPLE; i++) {
        person_t new_person;
        get_person_info(&new_person);
    }

Creates a person_t object on the stack and fills the new_person object with data, because the loop only does that, the object goes out of scope on the next loop iteration and the data is lost.

Using malloc :

for (int i = 0; i < NUM_PEOPLE; i++) {
        person_t *new_person = malloc(sizeof(person_t));
        get_person_info(new_person);
    }

Creates a person_t object on the heap and fills it with data, because its allocated on the heap the new_person object will outlive the loop scope which currently means that you're leaking memory because you have no pointer pointing at the data of the person_t object of the previous loop cycle.

Hatted Rooster
  • 35,759
  • 6
  • 62
  • 122
1

Both ways are correct !!

person_t *new_person = (person_t *) malloc(sizeof(person_t)); and then change get_person_info() to accept a person_t ** instead of a person_t *?

you don't need to change parameter of function -void get_person_infperson_t *person);.Just pass pointer to it in main like this -

get_person_info(new_person);

But in previous way without allocating memory , you won't be able to use it outside the block it is defined in whereas if your program depend on its life you can allocate memory to it on heap.

In your code you posted new_person is used inside loop only so if you don't intend to use to outside loop you probably won't need dynamic allocation .

But if you want to use it outside loop also you should use dynamic allocation. But don't forget to free it.

ameyCU
  • 16,489
  • 2
  • 26
  • 41
1

Not sure whether or not to malloc memory for a struct?

The short answer is: no need to do it in your case. If you want to use your object outside the forloop you could do it by dynamically allocated memory, namely:

person_t *new_person = malloc(sizeof(person_t));

and then call it with:

 get_person_info(new_person);

In you example, the object is used within the loop, thus there is no need to do it.

Note:

when you use dynamically allocated memory you should always free it, at the end to avoid memory leaks.

Edit:

As pointed out by @Johann Gerell, after removing the redundancy of the casting of the return type of malloc, in C, the allocation would look like:

person_t *new_person = malloc(sizeof(person_t));

malloc returns a void pointer (void *), which indicates that it is a pointer to a region of unknown data type. The use of casting is required in C++ due to the strong type system, whereas this is not the case in C.

Ziezi
  • 6,375
  • 3
  • 39
  • 49
1

Your confusion stems from not understanding object storage duration and pointers well. Let's see each one separately to get some clarity.

Storage Duration

An object can have automatic or dynamic storage duration.

Automatic

Automatic, as the name says, would be managed by the compiler for you. You just define a variable, use it and when it goes out of scope the object is destroyed automatically for you. A simple example:

if (flag) {
    int i = 0;
    /* some calc. involving i */
}
// i is dead here; it cannot be accessed and its storage is reclaimed

When the control enters the if's scope, memory large enough to hold an int will be allocated automatically and assigned the value 0. Once your use of i is over, when the control exits the scope, the name i goes out of scope and thus will no longer be accessible by the program and also its storage area allocated automatically for you would be reclaimed.

Dynamic

Lets say you want to have objects dynamically allocated i.e. you want to manage the storage and thereby the lifetime of the object without the scope or the compiler coming in your way, then you'd go on by requesting storage space from the platform using malloc

malloc(sizeof(int));

Notice that we're not assigning the return value of malloc to any pointer as you're used to seeing. We'll get to pointers in a bit, lets finish dynamic objects now. Here, space large enough to hold an int is handed over to you by malloc. It's up to you to free it when you're done with it. Thus the lifetime of this unnamed int object is in your hands and would live beyond the scope of the code that created it. It would end only when you explicitly call free. Without a matching free call getting called, you'd have the infamous memory leak.

Pointers

A pointer is just what its name says - an object that can refer to another object. A pointer is never what it is pointing at (pointee). A pointer is an object and its pointee is another separate, independent object. You may make a pointer point to another named object, unnamed object, or nothing (NULL).

int i = 0;
int *ptr1 = &i;                  // ptr1 points to the automatic int object i
int *ptr2 = malloc(sizeof(int)); // ptr2 points to some unnamed int object
int *ptr3 = NULL;                // ptr3 points to nothing

Thus the reason most people confuse pointers for dynamically allocated pointees comes from this: the pointee, here, doesn't have a name and hence they're referred to always via their pointers; some people mistake one for the other.

Function Interface

The function taking a pointer is appropriate here, since from the caller's viewpoint it's a flexible function: it can take both automatic and dynamic objects. I can create an automatic variable and pass it in, or I can pass a dynamic variable too:

void get_person_info(person_t *person);

person_t o { };
get_person_info(&a);

person_t *p = malloc(sizeof(person_t));
get_person_info(p);
free(p);

Is it necessary to malloc() memory for new_person within main()?

No. You can define an automatic variable and pass it to the function. In fact it's recommended that you try to minimize your usage of dynamic objects and prefer automatic objects since

  • It minimizes the chances of memory leaks in your code. Even seasoned programmers miss calling the matching free to a malloc thereby introducing a memory leak.
  • Dynamic object allocation/deallocation is far slower than automatic variable allocation/deallocation.
  • A lot of dynamic allocation deallocation causes memory fragmentation.

However, automatic variables are generally allocated in the stack and thus the upper limit on the number and size on how much you can create on the stack is relatively lower than what you can allocate dynamically (generally from the heap).

change get_person_info() to accept a person_t ** instead of a person_t *?

No, if you did so, the option of passing automatic variables would still be possible but cumbersome:

void foo(int **o);

int i = 0;
int *p = &i;  // p is redundant
foo(&p);

int *p = malloc(sizeof(int));
foo(&p);

As opposed the simpler

void bar(int *o);

int i = 0;
bar(&i);

int *p = malloc(sizeof(int));
bar(p);
Community
  • 1
  • 1
legends2k
  • 31,634
  • 25
  • 118
  • 222