Is returning a pointer to a static local variable safe?

I'm working with some code that widely uses the idiom of returning a pointer to a static local variable. eg:

char* const GetString()
{
  static char sTest[5];
  strcpy(sTest, "Test");
  return sTest;
}

Am I right in thinking that this is safe?

PS, I know that this would be a better way of doing the same thing:

char* const GetString()
{
  return "Test";
}

Edit: Apologies, the function signature should of course be:

const char* GetString();

First example: Somewhat safe

char* const GetString()
{
  static char sTest[5];
  strcpy(sTest, "Test");
  return sTest;
}

Although not recommended, this is safe, the scope of a static variable remains alive even when the scope of the function ends. This function is not very thread-safe at all. A better function would get you to pass a char* buffer and a maxsize for the GetString() function to fill.

In particular, this function is not considered a reentrant function because reentrant functions must not, amongst other things, return the address to static (global) non-constant data. See reentrant functions.

Second example: Completely unsafe

char* const GetString()
{
  return "Test";
}

This would be safe if you did a const char *. What you gave is not safe. The reason is because string literals can be stored in a read only memory segment and allowing them to be modified will cause undefined results.

char* const (const pointer) means that you can't change the address the pointer is pointing to. const char * (pointer to const) means that you can't change the elements that this pointer is pointing to.

Conclusion:

You should consider either:

1. If you have access to the code then modify the GetString to take a parameter of a char* buffer to fill and a maxsize to use.

2. If you do not have access to the code, but you must call it, wrap this method in another function which is protected by a mutex. The new method is as described in 1.

static variables (in a function) are like scoped global variables. In general, they should be avoided (like global variables, they cause re-entrancy issues), but are useful at times (some standard library functions use them). You can return pointers to global variables, so you can return pointers to static variables as well.

It depends on what you mean by safe. There are a couple of problems that I can see immediately:

1. You've returned a char * const, which will allow callers to change the string at this location. Potential buffer overrun. Or did you mean a const char *?
2. You might have a problem with reentrance, or with concurrency.

To explain the second, consider this:

const char * const format_error_message(int err)
{
    static char error_message[MAXLEN_ERROR_MESSAGE];
    sprintf(error_message, "Error %#x occurred", err);
    return error_message;
}

If you call it like this:

int a = do_something();
int b = do_something_else();

if (a != 0 && b != 0)
{
    fprintf(stderr,
        "do_something failed (%s) AND do_something_else failed (%s)\n",
        format_error_message(a), format_error_message(b));
} 

...what's going to be printed?

Same for threading.

Fundamentally, yes, it is safe in the sense that the value will last indefinitely because it is static.

It is not safe in the sense that you've returned a constant pointer to variable data, rather than a variable pointer to constant data. It is better if the calling functions are not allowed to modify the data:

const char *GetString(void)
{
    static char sTest[5];
    strncpy(sTest, "Test", sizeof(sTest)-1);
    sTest[sizeof(sTest)-1] = '\0';
    return sTest;
}

In the simple case shown, it is hardly necessary to worry about buffer overflows, though my version of the code does worry, and ensures null termination. An alternative would be to use the TR24731 function strcpy_s instead:

const char *GetString(void)
{
    static char sTest[5];
    strcpy_s(sTest, sizeof(sTest), "Test");
    return sTest;
}

More importantly, both variants return a (variable) pointer to constant data, so the user should not go modifying the string and (probably) trampling outside the range of the array. (As @strager points out in the comments, returning a const char * is not a guarantee that the user won't try to modify the returned data. However, they have to cast the returned pointer so it is non-const and then modify the data; this invokes undefined behaviour and anything is possible at that point.)

One advantage of the literal return is that the no-write promise can usually be enforced by the compiler and operating system. The string will be placed in the text (code) segment of the program, and the operating system will generate a fault (segmentation violation on Unix) if the user attempts to modify the data that is pointed to by the return value.

[At least one of the other answers notes that the code is not re-entrant; that is correct. The version returning the literal is re-entrant. If re-entrancy is important, the interface needs to be fixed so that the caller provides the space where the data is stored.]

Yes, it's perfectly safe. The lifetime of local statics are that of the entire program execution in C. So you can return a pointer to it, since the array will be alive even after the function returns, and the pointer returned can validly de-referenced.

It is very useful, as you can use the function directly as printf parameter. But, as it was mentioned, multiple calls to the function inside a single call will cause a problem, because the function uses the same storage and calling it twice will overwrite the returned string. But I tested this piece of code and it seems to work - you can safety call a function, where givemestring is used at most MAX_CALLS times and it will behave correctly.

#define MAX_CALLS 3
#define MAX_LEN 30

char *givemestring(int num)
{
        static char buf[MAX_CALLS][MAX_LEN];
        static int rotate=0;

        rotate++;
        rotate%=sizeof(buf)/sizeof(buf[0]);

        sprintf(buf[rotate],"%d",num);
        return buf[rotate];

}

The only issue is thread-safety, but this can be solved with thread local variables (gcc's __thread keyword)

Yes, this is used frequently to return the text portion of some lookup, i.e. to translate some error number into a human friendly string.

Its wise to do this in instances where you'd:

fprintf(stderr, "Error was %s\n", my_string_to_error(error_code));

If my_string_to_error() returned an allocated string, your program would leak given the above (very) common usage of such a function.

char const *foo_error(...)
{
    return "Mary Poppins";
}

... is also OK, some brain dead compilers might want you to cast it though.

Just watch strings in this fashion, don't return a book :)