return a inline declaration of string with char *

Question

First hallo to everyone, first post here.

Today I found myself wondering if this code is correct, it was written time ago by a friend for a microcontroller and I have no chance to ask him anymore.

The program works correctly, but I can't really decide if it works by luck or not (don't belive in luck with programming). I don't even know if the title of this post is correct, sorry if it could be misleading. The code:

char  *textStatusErrorMessage( unsigned int codeStatus )
{
    switch ( codeStatus ) {

    case STATUS_1:
        return ( (char  *) " Status: 1 " );
        break;     
    case STATUS_2:
        return ( (char  *) " Status: 2 " );
        break;      
    default:
        sprintf( tmpBuf, " UNKNOWN STATUS   %03d ", codeStatus );
        return ( (char  *) tmpBuf ); //tmpBuf is global
        break;
    }
}

more precisely this syntax is kind of obscure to me.

return ( (char  *) " Status: 1 " );

It returns a char * of what ? where is the "Status: 1 " string saved ? heap/stack???

As it's implemented the string is in the scope of the function and I'm assuming that after leaving the function with the return statement I have no control of what is gonna be written in the pointer returned by the function itself.

From the way I see it I would have had a global array with the different possible string-options and returned a pointer to the correct one selected by the CASE. So I know the pointer I return is to a well defined memory area.

So is this code wrong or not? Which is the most correct solution?

Thanks DAN

Good Read: [“life-time” of string literal in C](http://stackoverflow.com/questions/9970295/life-time-of-string-literal-in-c/9970305#9970305) — Alok Save, Jan 28 '13 at 15:56

score 6 · Accepted Answer · answered Jan 28 '13 at 15:50

6

In theory, this code is valid; string literals (e.g. " Status: 1 ") have a type of char[], and a lifetime equal to that of the whole program.

However, there are a few issues:

In the string literal cases, if anything tries to modify the returned string, then undefined behaviour occurs. Really, this function should return a const char * instead.
It's not clear where tmpBuf is declared, nor whether it points to sufficient memory to hold the data being written to it.
There is only one tmpBuf, so every time this function is called, tmpBuf will be overwritten. This is likely to cause hard-to-track bugs.
The casts are unnecessary.

answered Jan 28 '13 at 15:50

Oliver Charlesworth

267,707
33
569
680

Oli, I think the cast was absolutely necessary in order to shut up the compiler complaining about type mismatch between `const char *` and the return type `char *`. Of course this was not the right way to do it ;-) – junix Jan 28 '13 at 16:01
@netcoder Right. I just checked it for MinGW and it doesn't care about "nonconst". I thought I've seen compilers complaining about that but I must have been mistaken. – junix Jan 28 '13 at 16:55
Thanks very exaustive answer. – dfacchin Jan 29 '13 at 08:37

score 3 · Answer 2 · answered Jan 28 '13 at 15:51

It returns a char * of what ?

It casts the char[] that the string literal is to a char*. That isn't necessary, since it is automatically converted to a char* there.

where is the "Status: 1 " string saved ? heap/stack???

Typically, string literals are stored in the data (or rodata) segment of a programme. But that isn't important, what's relevant is that string literals have static storage duration, so the code is valid and the returned pointers don't point to local variables that don't exist after the function returns.

return a inline declaration of string with char *

2 Answers2