Calloc inside function

Question

Looking at this question that has just been asked: Inconveniences of pointers to static variables would doing something like this be considered bad practice, then?

char* strpart(char* string, int start, int count)
{
    char* strtemp; 
    int i = 0; int j = 0;
    int strL = strlen(string);

    if ( count == 0 )
    {
        count = strL;
    }

    strtemp = (char*) calloc((count + 1), sizeof(char));
    for ( i = start; i < (start+count); i++ )
    {
        strtemp[j] = string[i];
        j++;
    }
    return strtemp;
}

Sorry it's written quickly, but the basic principle is - when NOT using a static buffer inside a function is it bad practice to assign memory inside a function? I assume so because it wouldn't be freed, would it? Thought I ought to ask though.

score 7 · Accepted Answer · answered Feb 09 '10 at 18:34

7

It's not bad pratice but it can easily create memory leaks (the callers have to remember to free the memory).

One thing I like to do is use a naming convention to indicate what functions can allocate. For example, I'd name that function:

char* strpart_alloc(char* string, int start, int count)

answered Feb 09 '10 at 18:34

R Samuel Klatchko

74,869
16
134
187

One of the places where hungarian-style notation can really make a difference (if the dev even knows what the `_alloc` means..) – Dana the Sane Feb 09 '10 at 18:36
I like that idea. Ok so the correct method would then be to use char* ptr = strpart_alloc(...); do stuff with ptr; free(ptr)? – Feb 09 '10 at 18:37
@Ninefinger - yes, that is the correct way to call such a function. – R Samuel Klatchko Feb 09 '10 at 18:37

score 4 · Answer 2 · answered Feb 09 '10 at 18:33

It's always OK practice to allocate memory dynamically inside a function PROVIDED you return a pointer to that memory to the outside world, so that something else can free it, or free it yourself within the function.

score 2 · Answer 3 · answered Feb 09 '10 at 18:32

2

Well, it's dangerous. I'd try to avoid it when possible.

Your assumption is correct - the memory will not get released automatically.

The problem is that the return value here is memory allocated on the heap, the caller of your function has to remember to free. You're allocating memory here that will not (by you) get released. It's always a bad idea to put constraints on the user of your API.

Sometimes (rarely) this can't be avoided, so if you do this, make sure to document it very clearly.

answered Feb 09 '10 at 18:32

Reed Copsey

554,122
78
1,158
1,373

Ok thanks, I didn't think it would get freed done like this so that's nice to know. Given the answers to the previous question it's fairly trivial to write a safe alternative anyway. Thanks again. – Feb 09 '10 at 18:34

score 2 · Answer 4 · answered Feb 09 '10 at 18:35

2

It's common to do this. You just have to clearly note in your documentation "API" that it is the caller's responsibility to free the returned pointer when finished.

answered Feb 09 '10 at 18:35

Nathan Kidd

2,919
21
22

score 2 · Answer 5 · answered Feb 09 '10 at 18:41

It is not a bad practice. The fact that the function returns a malloc-ed (or calloc-ed) memory becomes a part of its external specification. It becomes a the caller's responsibility to free it when it is no longer necessary.

It is inelegant though. It is inelegant since it 1) forces the use of dynamic memory, when the caller might prefer to avoid it, and 2) forces the use a specific kind of dynamic memory - the malloc-ed one, when the caller might prefer to use its own allocation mechanism.

Calloc inside function

5 Answers5

Linked