Malloc memory to char * in function. Return *char or pass char ** as function parameter?

Question

char *a = NULL;
char *b = NULL;

Method 1:

char *malloc_string(int string_size){
    return (char*)malloc(string_size);
}

a=malloc_string(4);

free(a);

Method 2:

void malloc_string(char **a, int string_size){
    *a = (char*)malloc(string_size);
}

malloc_string(&b, 4);

free(b);

Which would you choose and why?

BTW don't cast the return value of malloc in C, so actually the first method is useless, just use `a = malloc(4)` instead of `a = malloc_string(4)` — Jabberwocky, Jul 22 '15 at 11:49
A common reason for using the pattern in method 2 is for the function to return an error code, rather than being `void`. — paddy, Jul 22 '15 at 11:51

score 2 · Answer 1 · answered Jul 22 '15 at 11:53

2

Neither: both are terrible.

Don't have stub functions for malloc since your code becomes less clear to folk reading your code. (malloc is a standard function and everyone knows what it does.)
And don't cast malloc on the right hand side of =: it's bad programming style since (i) you are repeating yourself and (ii) someone refactoring your code might change the pointer type in one and not the other place. That can introduce undefined behaviour.

answered Jul 22 '15 at 11:53

Bathsheba

231,907
34
361
483

So, malloc within a callable function is bad? You mean you would malloc for pointer first and then pass the pointer to function? – Jul 22 '15 at 11:58
No that's fine. But I'd never stub `malloc` for the sake of it. Just don't lose track of the returned pointer. – Bathsheba Jul 22 '15 at 12:00

score 1 · Answer 2 · edited May 23 '17 at 12:14

1

To answer this question, TL;DR, any one. Both are OK.

To nitpick, NONE, because

Both the methods lack error check. What's the use of the wrapper then?

1.1. If I pass string_size as a -ve value, malloc() will blow up.

1.2. malloc() failure (or, success) should be checked before using the returned pointer.
Please see why not to cast the return value of malloc() and family in C.

edited May 23 '17 at 12:14

Community

1
1

answered Jul 22 '15 at 11:49

Sourav Ghosh

133,132
16
183
261

Thanks! Of course in practice, I will add checks on parms, this piece of code is just to describe the question briefly. – Jul 22 '15 at 12:03
@AttufliX That's why I mentioned my statements in "nitpick". :-) – Sourav Ghosh Jul 22 '15 at 12:05

score 1 · Answer 3 · edited May 23 '17 at 11:51

The correct way in C would be to never define an malloc_string() in the first place, it just introduces an unnecessary (and, hopefully optimized-away) function call. Just do this:

a = malloc(4);

See also Do I cast the result of malloc?

Apart from that, your argument type is wrong, it must be size_t, not int (and your compiler should warn you about this, enable compiler warnings)

Finally, you should probably define a single wrapper around malloc to handle errors in a central place ... in it's simplest form, it would look like this:

void *xmalloc(size_t size)
{
    void *ret;
    if (!(ret = malloc(size))
    {
        /* perror("malloc"); */
        exit(1);
    }
    return ret;
}

You are right, malloc receives size_t params. I think enabling compiler warnings is a good habit. — , Jul 22 '15 at 12:25

score 0 · Answer 4 · answered Jul 22 '15 at 17:00

The only thing is worth substituting is free(), the C standard doesn't say that after freeing the memory the pointer has to be NULLed;

#define SAFE_FREE(m)    \
{                       \
   if(m)                \
   {                    \
      free(m);          \
      m = NULL;         \
   }                    \
}

A good practice is to NULL the freed pointer, that will help to catch potential problems earlier aspesially if you have a habit to use assert for every internal functions, like this:

void foo_internal(void* p)
{
   assert(p != NULL);

   // ...
}

If you do not NULL the pointer after freeing it and call the func the assert will never be triggered on the most compilers and platforms (memory allocation are platform specific).

That might seem not a big deal if you have IDE debugger and can step through the code but in some cases, like UEFI, embedded, etc. where you don't have a debugger that simple practice would save a lot of headache!

Yeah, not properly free the pointer or use an already freed pointer may cause many headache problems. Setting NULL to freed yet pointer and using assert is really a good habit. Tks! — , Jul 22 '15 at 17:11

Malloc memory to char * in function. Return *char or pass char ** as function parameter?

4 Answers4