21

I notice that MS compilers give "deprecated" warnings for cstdlib functions like getenv. MS has invented its own standard such as _dupenv_s.

Question 1

AFAIK the main "unsafe" thing is about reentrancy *. Since MS's CRT is marked as "multi-threaded" (/MT), why don't they just replace getenv with the reentrant, thread-safe version? Is it like anybody would depend on the unsafe behavior?

Question 2

I compiled the same code with GCC g++ -Wall -Wextra -Weff++ -pedantic foo.cpp and it doesn't yield any warnings. So I guess this is not a problem on POSIX? How is this solved? (OK maybe they just changed the behavior of getenv, would be nice to have this confirmed).

* It's an over-generalization to say that its' only about reentrancy. Of course we have things like strncpy_s which changes the signature completely and deals with buffer size. But doesn't change the core of this question

kizzx2
  • 18,775
  • 14
  • 76
  • 83
  • 5
    IMO, g++ simply doesn't think it is its duty to inform you of "unsafe" functions. As far as VC++ goes, practically the entire C standard library is deprecated as unsafe. – UncleBens Nov 26 '10 at 16:30
  • But the difference between the functions you mention is that in one case it is your responsibility to `free` the returned pointer. – UncleBens Nov 26 '10 at 16:46
  • 1
    @UncleBen: I think that's not true. I just confirmed with MS's `crtdbg.h` and Valgrind on Linux for a simple one line program `int main(){getenv("PATH");}` and both reported no memory leaks :P In fact, `int main(){free(getenv("PATH"));}` would yield an assertion error. – kizzx2 Nov 26 '10 at 16:53
  • 4
    VC++'s `getenv` looks to be thread-safe already (you can check the source for yourself, it's in `getenv.c`); I imagine the deprecation is due to the non-`const` return value, which leaves open the possibility of writing past the end of an internal buffer without the type system even warning you about the possibility. But this is only a guess, because there doesn't seem to be any documented rationale for any of this... –  Nov 26 '10 at 17:23

3 Answers3

23

  1. In a sane world, the answer would be "of course not, that would be stupid!" In this world, though, it seems there is no end of gut-wrenchingly poorly thought out undocumented behavior upon which people will stoop to depending upon. Raymond Chen has a great collection of such anecdotes (anecdon'ts?) in his blog. Such as the hideous practice of using a bug in the loader to share thread-local variables between an exe and a DLL. When you have as many customers as Microsoft does, the only safe choice is to never even risk breaking backwards compatibility.

  2. The difference in warnings is because cl.exe is going out of its way to highlight a potential security problem, and g++ isn't. getenv and puts and friends are all still broken under POSIX, but (at least for getenv) there isn't a more secure alternative in the standard library. And, unlike Microsoft, the GNU folks probably see a standard library call with potential security problems as a lesser evil than a more secure but platform-specific library call.

Ben Karel
  • 4,591
  • 2
  • 26
  • 25
9

It annoys the heck outta me that Microsoft chose to do this. I know how to call all the functions safely, I don't want or need these extra warnings.

Just set _CRT_SECURE_NO_WARNINGS and be done with it. It's really that silly.

Joshua
  • 40,822
  • 8
  • 72
  • 132
  • 6
    Probably the same reason they added _"Are you sure you want to perform ? Yes/No"_ messages. People are.. well, not that smart all the time :) – default Nov 26 '10 at 17:44
7

For the specific case of getenv, it is indeed not reentrant or thread safe. As for why Microsoft doesn't just replace it, you can't take that interface and make it reentrant (you can almost make it "thread safe" with thread local storage, but it would still not be reentrant).

Even if you just took getenv away altogether, there is still the problem that you have the environ variable which would require some serious compiler level support to make thread safe, since it is just data.

Really, using environment variables for anything other than "setting it up before the process starts or at process start, and only reading from it from that point on" is going to probably end in tears if you have more than one thread. setenv and putenv don't have a rich enough interface to express something like "set this set of environment variables atomically" and likewise getenv doesn't have a way to express "read this set of environment variables atomically".

_dupenv_s is somewhat silly in my opinion, because if using that suddenly makes your code safe, it could probably done in a safe way with getenv. _dupenv_s solves a tiny subset of the problems with using environment variables in a multithreaded scenario.

Logan Capaldo
  • 39,555
  • 5
  • 63
  • 78
  • 4
    Nice point. Following your logic -- MS's deprecated warning is really redundant. Take `getenv` for example: 1) `GetEnvironmentVariable` probably doesn't solve reentrancy, yet it is not "deprecated." 2) If I really want to secure my code and go to implement a Ph.D grade thread-safe rigorous-algorithm wrapper, it's still going to call `getenv` at some point, so I'm still unsafe by MSVC's definition. (Unless I alienate the standard and succumb to the MS-specific realm) – kizzx2 Nov 26 '10 at 17:02