views:

820

answers:

6

I'm porting some code to Windows, and the Microsoft compiler (Visual C++ 8) is telling me that strerror() is unsafe.

Putting aside the annoyance factor in all the safe string stuff from Microsoft, I can actually see that some of the deprecated functions are dangerous. But I can't understand what could be wrong with strerror(). It takes a code (int), and returns the corresponding string, or the empty string if that code is not known.

Where is the danger?

Is there a good alternative in C?

Is there a good alternative in C++?

[edit]

Having had some good answers, and now understanding that some implementations may be crazy enough to actually write to a common shared buffer - unsafe to reentrancy within a single-thread, never mind between threads! - my question stops being "Why can't I use it, and what are the alternatives?" to "Are there any decent, succinct alternatives in C and/or C++?"

Thanks in advance

+7  A: 

strerror deprecated because it is unsafe and not thread-safe. strerror work on a internal static buffer. You must use more secure variant called strerror_s.

The secure variant require that the buffer size be passed to the function in order to validate that the buffer is large enough before writing to it, helping to avoid dangerous buffer overrun errors that could allow malicious code to execute.

dfa
Er, the question was: _Why_ it is considered unsafe? You've told me precisely nothing.
JamieH
He _did_ tell you why. It works on an internal static buffer - that is, one that is shared across threads. That's unsafe.
nsayer
Ah, yes. My apologies. Now, the next question - to the world at large - is: why in the heck would anyone implement it like that!?
JamieH
JamieH, the function could be efficiently implemented as char *msg[] = { "file not found", "you are ill", "you are't root", "time is over" }; char *strerror(int n) { return msg[n]; }
Johannes Schaub - litb
"It compiles, ship it!"
Andrew Coleson
litb - I figured it would be like that (though with a bit more sophistication to match code with string)Andrew C - LOL!
JamieH
@litb and @JamieH: the simplest implementation is only marginally more sophisticated than the outline. One problem comes with locales - if the messages need to be localized, etc.
Jonathan Leffler
It's all about locales - the string returned from strerror needs to remain valid until strerror is called again, even if you change locale (which might unload the old locale). So returning a pointer into the locale data means the implementation needs to keep the old locale hanging around. I assume it's doable, but not as simple as you'd think.
Steve Jessop
indeed, i was posting only the very rough plan. obviously it's more complicated in RealLife :)
Johannes Schaub - litb
+4  A: 

You can not rely on the string that is returned by strerror() because it may change with the next call to the function. The previously returned values may become obsolete then. Especially in multi-threaded environments, you can not ensure that the string is valid when you access it.

Imagine this:

Thread #1:
char * error = strerror(1);
                                    Thread #2
                                    char * error = strerror(2);
printf(error);

Depending on the implementation of strerror(), this code prints out the error code for error code 2, not for error code 1.

beef2k
By what mechanism would it change? Presumably the strings are defined - statically - in the internals of the implementation of the C runtime library, and thus will never change? Or is that wrong, and they may indeed change on a dynamic basis?
JamieH
Why would you assume this? Yes, it COULD be done like this, but it COULD be done completely different - we don't know ;) ..
beef2k
The main problem is that Posix doesn't require strerror() to be thread-safe. You have to use strerror_r() instead. On Windows use strerror_s().
Bastien Léonard
+1  A: 

Although I do not know Microsoft's reasons, I note that strerror returns a non-const char *, which means that there is a risk that some Merry Prankster has called strerror before you did and modified the message.

Thomas L Holaday
If if it's "const char *" the same Merry Prankster might try to cast to (char *) and change the code after that ;) .. Declaring something as "const" does not mean that it cannot be changed. It just gives an optimization hint to the compiler.
beef2k
I agree that, from a pure const-ness perspective, this is true, but I strongly suspect that the lack of const is just historical, and users are obliged not to alter the contents of the string in the same way as they are with many such non-const-but-should-be parts of the standard library. _If_ that is the case, then I can still see no reason for strerror()'s deprecation.
JamieH
making it const char* and saying the thing must not be changed would be enough to make it thread safe *if* you make sure you have synchronized the calls so that uses of it don't interleave. In any case, you can't forbid the programmer to make his program crash. He could still create an array and write out of array bounds. and could kill the other threads using Terminate or something :) Making the return a const char* is an effective protection against accidental changes.
Johannes Schaub - litb
Real Programmers put static consts in ROM.
Thomas L Holaday
+5  A: 

strerror by itself is not unsafe. In the olden days before threading it simply wasn't a problem. With threads, two or more threads could call strerror leaving the returned buffer in an undefined state. For single threaded programs it shouldn't hurt to use strerror unless they're playing some weird games in libc, like common memory for all apps in a DLL.

To address this there's a new interface to the same functionality:

int strerror_r(int errnum, char *buf, size_t buflen);

Note that the caller provides the buffer space and the buffer size. This solves the issue. Even for single threaded applications you might as well use it. It won't hurt a bit, and you might as well get used to doing it the safer way.

NOTE: the above prototype is the XSI spec. It may vary per platform or with compiler options or #define symbols. GNU, for instance, makes that or their own version available depending on a #define

dwc
And there's also TR24731 which defines strerror_s().
Jonathan Leffler
"In the olden days before threading it simply wasn't a problem". It's not re-entrant either, but then it never claimed to be callable from signal handlers.
Steve Jessop
+2  A: 

For a succinct wrapper, you can use STLSoft's stlsoft::error_desc, as in:

std::string errstr = stlsoft::error_desc(errno);

Looking at the code, it seems that it's implemented in terms of strerror(), which means it'll be safe for reentrancy within a thread (i.e. if used multiple times within a given statement), but it does not address the multithreading problem.

They seem to operate pretty rapid release cycles for defects, so you could try requesting a mod?

dcw
Thanks. I might try that.
JamieH
+2  A: 

Having had some good answers, and now understanding that some implementations may be crazy enough to actually write to a common shared buffer - unsafe to reentrancy within a single-thread, never mind between threads! - my question stops being "Why can't I use it, and what are the alternatives?" to "Are there any decent, succinct alternatives in C and/or C++?"

Posix specifies strerror_r(), and on Windows you can use strerror_s(), which is a bit different but has the same goal. I do this:

#define BAS_PERROR(msg, err_code)\
  bas_perror(msg, err_code, __FILE__, __LINE__)

void bas_perror (const char* msg, int err_code, const char* filename,
                 unsigned long line_number);


void
bas_perror (const char* usr_msg, int err_code, const char* filename,
            unsigned long line_number)
{
  char sys_msg[64];

#ifdef _WIN32
  if ( strerror_s(sys_msg, sizeof sys_msg, err_code) != 0 )
  {
    strncpy(sys_msg, "Unknown error", taille);
    sys_msg[sizeof sys_msg - 1] = '\0';
  }
#else
  if ( strerror_r(err_code, sys_msg, sizeof sys_msg) != 0 )
  {
    strncpy(sys_msg, "Unknown error", sizeof sys_msg);
    sys_msg[sizeof sys_msg - 1] = '\0';
  }
#endif

  fprintf(stderr, "%s: %s (debug information: file %s, at line %lu)\n",
          usr_msg, sys_msg, filename, line_number);
}

I wrote this function because Posix threads functions don't modify errno, they return an error code instead. So this function is basically the same as perror(), except that it allows you to provide an error code other than errno, and also displays some debugging information. You can adapt it to your need.

Bastien Léonard