CODEWITHSUNDEEP
coptimization
ant2009
ant2009

Reputation: 22486

Returning a string in a function

gcc 4.7.2
c89

Hello,

I have a function that will return a string based on the current state of a channel. Am just wondering what would be the better technique to use. The first I use a memory pool so that the memory will always be valid. The second just returning a string in the return statement. And the third using a local pointer allocated on the stack and returning that.

In all cases I am just print the string, and won't be using it for anything else.

Which one would you recommend?

Many thanks for any suggestions,

Using the function like this:

MODULE_LOG(PRIO_DEBUG, "%s|%s",
        g_channel_state_to_string(channel->previous_state, channel->mem_pool),
        g_channel_state_to_string(channel->current_state, channel->mem_pool));

Using a memory pool passed in

static char* g_channel_state_to_string(states_e state, apr_pool_t *mem_pool)
{
    char *channel_state = NULL;

    switch(state) {
    case CHANNEL_IDLE:
        channel_state = apr_pstrdup(mem_pool, "CHANNEL_IDLE");
        break;

    default:
        channel_state = apr_pstrdup(mem_pool, "CHANNEL_UNKNOWN_CHANNEL_STATE");
        break;
    }

    return channel_state;
}

Returning the string in the return statement

static char* g_channel_state_to_string(states_e state)
{  
    switch(state) {
    case CHANNEL_IDLE:
        return "CHANNEL_IDLE";
        break;

    default:
        return "CHANNEL_UNKNOWN_CHANNEL_STATE";
        break;
    }
}

Assign the string literal to a local pointer could result in a stack dump as the memeory is allocated on the stack and may not exist when the function returns.

static char* g_channel_state_to_string(states_e state)
{
    char *channel_state = NULL;

    switch(state) {
    case CHANNEL_IDLE:
        channel_state = "CHANNEL_IDLE";
        break;

    default:
        channel_state = "CHANNEL_UNKNOWN_CHANNEL_STATE";
        break;
    }

    return channel_state;
}

Upvotes: 0

Views: 108

Answers (1)

Roman Saveljev
Roman Saveljev

Reputation: 2594

Returning the string in the return statement and Assign the string literal to a local pointer.. are identical solutions. Using a memory pool passed in is more error prone, unless you change function prototype to return const char*.

In the third option you assign address of "CHANNEL_IDLE" literal to the local variable of pointer type, which is further returned by value. So, the primitive address gets returned, which stays valid regardless of stack (see explanation below). Option three is identical to return 0xffffaaaa;

String literals defined throughout the code will be carefully scavenged by the linker and relocated into a single code section. Depending on your platform in the runtime loader can actually place it into read-only memory and write access can generate hardware abort. Option one makes returned string modification viable, because it's copy will sit in the heap. From your usage example, I doubt you want string modification, so the 'strdup' overhead is needless.

Since options two and three are identical, I would change function prototype to have returned type as const char* and went for either of those.

P.S. "We should forget about small efficiencies, say about 97% of the time: premature optimization is the root of all evil" D. Knuth

Upvotes: 4

Related Questions