pointer as second argument instead of returning pointer?

Question

I noticed that it is a common idiom in C to accept an un-malloced pointer as a second argument instead of returning a pointer. Example:

/*function prototype*/    
void create_node(node_t* new_node, void* _val, int _type);

/* implementation */
node_t* n;
create_node(n, &someint, INT)

Instead of

/* function prototype */
node_t* create_node(void* _val, int _type)

/* implementation */
node_t* n = create_node(&someint, INT)

What are the advantages and/or disadvantages of both approaches?

Thanks!

EDIT Thank you all for your answers. The motivations for choice 1 are very clear to me now (and I should point out that the pointer argument for choice 1 should be malloc'd contrary to what I originally thought).

Answer 1

personally I like to return data using refernce or pointer params, and use the function return for returning error codes.

Answer 2

Accepting a pointer (which the caller is responsible for malloc'ing or not) to memory to be filled in, offers serious advantages in flexibility over returning a pointer (necessarily malloc'ed). In particular, if the caller knows it needs to use whatever's returned only within a certain function, it can pass in the address of a stack-allocated struct or array; if it knows it doesn't need reentrancy, it can pass in the address of a static struct or array -- in either case, a malloc/free pair gets saved, and such savings do mount up!-)

Answer 3

That doesn't make much sense. Pointers in C are passed by value, just like other objects—the difference lies in the value. With pointers, the value is the memory address, which is passed to the function. However, you're still duplicating the value, and so when you malloc, you'll be changing the value of the pointer inside your function, not the one on the outside.

void create_node(node_t* new_node, void* _val, int _type) {
    new_node = malloc(sizeof(node_t) * SIZE);
    // `new_node` points to the new location, but `n` doesn't.
    ...
}

int main() {
    ...
    node_t* n = NULL;
    create_node(n, &someint, INT);
    // `n` is still NULL
    ...
}

There are three ways to avoid this. The first is, as you mentioned, returning the new pointer from the function. The second is to take a pointer to the pointer, thereby passing it by reference:

void create_node(node_t** new_node, void* _val, int _type) {
    *new_node = malloc(sizeof(node_t) * SIZE);
    // `*new_node` points to the new location, as does `n`.
    ...
}

int main() {
    ...
    node_t* n = NULL;
    create_node(&n, &someint, INT);
    // `n` points to the new location
    ...
}

The third is to simply malloc n outside the function call:

int main() {
    ...
    node_t* n = malloc(sizeof(node_t) * SIZE);
    create_node(n, &someint, INT);
    ...
}

Answer 4

I usually prefer receiving pointers (property initialized) as function arguments as opposed to returning a pointer to a memory area that has been malloc'ed inside the function. With this approach you are making explicit that the responsibility of memory management is on the user's side.

Returning pointers usually leads to memory leaks, as it's easier to forget to free() your pointers if you didn't malloc()'ed them.

Answer 5

1) As Samir pointed out the code is incorrect, pointer is passed by value, you need **

2) The function is essentially a constructor, so it makes sense for it to both allocate the memory and init the data structure. Clean C code is almost always object oriented like that with constructors and destructors.

3) Your function is void, but it should be returning int so that it can return errors. There will be at least 2, probably 3 possible error conditions: malloc can fail, type argument can be invalid and possibly value can be out of range.

Answer 6

I usually don't make a fixed choice, I cleanly put it into its own library and provide the best of both worlds.

void node_init (node_t *n);

void node_term (node_t *n);

node_t *node_create ()
{
    node_t *n = malloc(sizeof *n);
    /* boilerplate error handling for malloc returning NULL goes here */
    node_init(n);
    return n;
}

void node_destroy (node_t *n)
{
    node_term(n);
    free(n);
}

For every malloc there should be a free, thus for every init there should be a term and for every create there should be a destroy. As your objects grow more complex, you will find that you begin to nest them. Some higher level object may use a node_t list for internal data management. Before freeing this object, the list must be freed first. _init and _term care for this, completely hiding this implementation detail.

There can be decisions about further details, e.g. destroy may take a node_t **n and set *n to NULL after freeing it.