Search code examples
cpointersscopereturn

best practice regarding char pointers returned by functions


What is best practice when dealing with functions which return malloc'd pointers to C-strings?

Here's an example:

FILE *f;
char *tmp;
for (i = 0; i <= 10; i++) {
    tmp = concat(fn, i);
    f = fopen(tmp, "w");
    free(tmp);
    // read f, do something useful ...
    fclose(f);
}

char* concat(char *a, int b) returns a pointer to a new C-string which contains the concatenation of a and b.

Not only do I have to specify a temporary pointer which is then passed to fopen, I also have to free(tmp) every time. I would rather prefer something like this:

FILE *f;
char *tmp;
for (i = 0; i <= 10; i++) {
    f = fopen(concat(fn, i), "w");
    // read f, do something useful ...
    fclose(f);
}

But this of course leads to memory leaks. So what is best practice here? Something like concat(char *a, int b, char *result) where result is expected to be a pre-allocated memory for the resulting C-string? This solution has its disadvantages like limited or not optimal size of result.


Solution

  • What is best practice when dealing with functions which return malloc'd pointers to C-strings?

    Best practice: don't use them. A library expecting the caller to free returned data is almost certainly badly designed, with very few exceptions. We know this from 40 years of C language history, where crappily written libraries have created millions upon millions of memory leak bugs.

    The basic rule of sane, useful library API design is:

    Whoever allocates something is responsible for cleaning up their own mess.

    Since C doesn't have RAII or constructors/destructors, that unfortunately means that the sane library needs to provide you with a function for the clean-up and you need to remember to call it. If it doesn't provide such a function, you might want to consider writing wrapper functions that do this - correcting the bad library design for them.

    If you are the one implementing the library, you should always try to leave memory allocation to the caller, whenever possible. This is traditionally done by the function taking a pointer to a buffer which it writes to. Then either leaves it to the caller to allocate enough memory (like strcpy/strcat), or alternatively provide a variable with maximum buffer size, after which the function returns how much of the buffer it actually used (like fgets).


    In your example, a soundly designed concat would perhaps look like

    const char* concat (char* restrict dst, const char* restrict src, int i);
    

    Where src is the source string, i is the integer to add and dst is a large-enough buffer provided by the caller. Optionally, the function returns a pointer equivalent to dst, for convenience. The above also implements proper const correctness plus a micro-optimization with restrict that means that the pointers passed aren't allowed to overlap.

    Usage:

    char buf [LARGE_ENOUGH];
    fp = fopen(concat(buf, foo, i), "w");