Assigning a char pointer to string literal generated at runtime

I am in the process of reviewing some code that someone else wrote. I came across an interesting case involving strings within this code and I need help understanding how this works.

There is a function designed to be exported to a DLL. At the top of the function, we have this declaration

char *msg; // pointer to char
int error = 0; //my error code

Then, later in the code, we invoke a special library function for the IDE we use:

if(error < 0)
msg = getErrorString(error);

This inbuilt library function (getErrorString) expects you to provide a pointer to char where it can store the generated error string at runtime.

Finally, the code author calls the below:

free(msg); // freeing dynamically allocated memory??

So, I guess there is memory allocated dynamically at runtime which is large enough to store the generated error string? How is this allowed without explicitly calling something like malloc? If I were writing equivalent code, my first instinct would be to declare some static array like msg[256] and then do something like:

char msg[256] = {""};
sprintf(msg, "%s", getErrorString(error));

So my main question is, how can you declare a pointer to char, then assign it to a string which is generated at runtime, as shown in the original code? It seems that memory is being dynamically allocated at runtime, perhaps by the runtime engine. Is this what's happening with this code? Is my static array method preferred in this case?

CodePudding user response：

How is this allowed without explicitly calling something like malloc?

Well, it is almost certainly the case that somewhere within the implementation of the getErrorString, there is a call to malloc or the equivalent.

It is likely that the implementation of getErrorString looks something like this:

char *getErrorString(int error)
{
    char *ret = malloc(25);
    if(ret == NULL) abort();
    switch(error) {
        case EMUCHMEM:
            strcpy(ret, "too much memory");
            break;

        case EIUNDERFLOW:
            strcpy(ret, "integer underflow");
            break;

        case EDIVBY1:
            strcpy(ret, "divide by 1");
            break;

        default:
            sprintf(ret, "error %d", error);
            break;
    }

    return ret;
}

If I were writing equivalent code, my first instinct would be to declare some static array like msg[256] and then do something like: char msg[256] = {""}; sprintf(msg, "%s", getErrorString(error));

That doesn't make much sense. It represents unnecessary extra memory (256 bytes in msg[]) and unnecessary extra copying (by sprintf).

So my main question is, how can you declare a pointer to char, then assign it to a string which is generated at runtime, as shown in the original code?

You can always declare a pointer to char, then assign to it a string which is generated at runtime.

This can be a confusing point, though. You may have heard that strings are represented as arrays of char in C — which is true — and you may also have heard that you can't assign arrays in C — which is also true. You may have heard that instead of directly assigning strings, you always have to call strcpy. But that's not necessarily true — you can also "assign" strings by simply assigning pointers, which is what's going on when you say msg = getErrorString(error). In other words, there are two completely different ways of assigning strings in C: by copying arrays, or assigning pointers. See this other answer for more on this point.

It seems that memory is being dynamically allocated at runtime

Yes, that's how it seems.

Is my static array method preferred in this case?

As other comments have suggested, dynamic memory allocation in this case may or may not be a good idea. As a general rule, though, dynamic memory allocation is a perfectly fine — more or less vital — technique. Static memory allocation, on the other hand, can have plenty of problems of its own.

CodePudding user response：

getErrorString must documents that it calls malloc. It's generally considered very bad practice to write an API where functions return pointers to allocated memory and then expect someone else to clean it up. We know from 40 years of C history that badly designed APIs like this is exactly how you create memory leaks all over the place.

A better API would provide an explicit init/create function and an explicit cleanup/delete function. What those functions do internally is none of the caller's business - they just need to ensure to call init before anything else and cleanup the last thing they do.

On the topic of s****y API design, I would expect a function getErrorString to return a const char*, because why would an error message need to be modified by the caller? This ought to be an immutable string.