The Old New Thing

How would someone know that the call to FreeEnvironmentStrings() did nothing? Do some people regularly disassemble API functions to see if it's OK to not call them or something?

"To fix this, the GetEnvironmentStrings function was changed to return a copy of the environment block even if you call the Unicode version."

When did that happen? It appears XP SP3 (+ all updates) still returns a direct pointer:

kernel32!GetEnvironmentStringsW:

7c812f98 64a118000000     mov     eax,fs:[00000018]

7c812f9e 8b4030           mov     eax,[eax+0x30]

7c812fa1 8b4010           mov     eax,[eax+0x10]

7c812fa4 8b4048           mov     eax,[eax+0x48]

7c812fa7 c3               ret

kernel32!FreeEnvironmentStringsW:

7c814b77 33c0             xor     eax,eax

7c814b79 40               inc     eax

7c814b7a c20400           ret     0x4

Ah irony. Q indirectly answers Karellen's question.

I still have bad memories of our architect doing similar things. The "who cares what MSDN says, their code doesn't do anything, so don't call it"..

Q:

32bit Vista the changes Raymond mentions take place as well as the 64bit editions as well.

Sorry, but if we'd have to read the MSDN page for every single method of every single class we have ever used (citing .NET Framework's 10.000+ classes as an example), we would not have finished writing any reasonably complex piece of software. That's why people are relying on conventions and recurring patterns in the blackbox code they use. Changing API implementations - after the fact - might seem correct for the engineer who designed the API (and knows his/her "own" MSDN page by heart), but not for the rest of us.

Anonymous Coward, please stop writing software if you think it's better to guess about what APIs do rather than read their contracts. Your software must only work by chance.

It's not like it takes long to push F1 and read about the function (at least if you're writing .Net and F1 still takes you to the function for your language/framework and not Windows CE, MFC or .Net).

I'd argue that it would take longer to experiment with the API to determine its behaviour than it would to simply skim the documentation.

Whenever the OS gives you a piece of data, especially when -- as is the case here -- we are talking unmanaged C/C++ and not .Net, the first thing you should be asking is "who owns this memory/object and how is it freed? The second thing should be "what are the error return values and/or behaviours. You don't get any of that by guessing.

Back to the root post: This is why I still call GlobalLock and GlobalUnlock even though they "don't do anything anymore".

@Anonymous Coward

Apples and oranges!  If we are writing in this blog then we are writing about the Win32 API, which is not .NET, so don't compare the two.

The collection of API calls, while not small, is manageable.  Let me rephrase, the collection of API calls *that matter* is manageable.  There is no reason to scoff at the MSDN and go on your own based on disassembly listings of Windows' DLL files.

Are these errors in the MSDN?  Sure, it happens.  However, and I cannot speak for .NET development, I have always been satisfied with the level of detail the MSDN provides for *Win32 API* functions.  I have never needed to disassemble a Win32 function to figure out what is going on.

The sentiment is great, but this seems like one of those cases where you're simply doomed if you want to change the API's implementation.  For all practical purposes, this is a compatibility break.

It's hard enough to make internal API changes that affect 100 other developers on your own team.

Sometimes I've been tempted to have an API implementation walk up the call stack and rewrite (using self-modifying code) the buggy calling code.  Never actually done that, though.

Do you still need to use Global or Local Memory functions?  I suppose there are a few system APIs which require memory allocated from them, but I'd stick with the CRT or the Heap* APIs for any other uses.

"Back to the root post: This is why I still call GlobalLock and GlobalUnlock even though they "don't do anything anymore"."

Don't GlobalLock and GlobalUnlock do something for GMEM_MOVEABLE memory?

I go further. I call UnlockResource, even though it's clearly marked as obsolete and not doing anything, and probably implemented as a macro doing nothing. I just feel that there's something wrong with locking something but not unlocking it. I'm not sure that it's available in latest Win32 SDK headers.

I'm curious. Do FreeEnvironmentStrings do anything in Windows 95/98/Me? Win32 != Windows NT/2000/XP! Assuming that Windows is necessarily based on the NT kernel is stupid!

"For all practical purposes, this is a compatibility break." - As Leo Davidson says, for <i>every</i> bit of data in C(++), you must always know who is responsible for freeing it, it is part of the language itself, and there is no way to represent this in the language itself so you have no choice. If you start with the assumption that people won't do this... well, you might as well not write the code at all, because they have to.

You just can't build APIs built on the assumption that the API users won't know the language the API is in. (This isn't an absolute, it's good to think about common mistakes, how to catch them and mitigate them, but at fundamental level you have to assume your API users know how the language works, because without that your API doesn't matter, those programmers are screwed no matter what.)

"you have to assume your API users know how the language works, because without that your API doesn't matter, those programmers are screwed no matter what"

Until you get blamed because your upgrade "caused" their program to stop working, at which point it is automatically your fault.

I have a lot of sympathy for Raymond's arguments in favor of compatibility.  It can be taken too far, though.

In this case, I would have suggested that the original API should *not* have had a Free function, and that rather than changing the existing API, we'd have to create a new API GetEnvironmentStrings2() (or whatever you want to call it).  But then we get into the mess that this was only an issue with the Unicode version of the function, and... well... yuck.  (And as for Windows's Unicode support strategy, which makes it painful to write code that handles Unicode and is portable across Windows/Linux/Mac, that's a topic for another day... oh, if only you could pass UTF-8 strings to the "A" functions rather than them using the current code page...)

You guys, who are married to the degenerate imperative programming mindset, have got it all backwards.

The simple fact is that the documented contract has always required people to call the FreeEnvironmentStrings function afterwards.  If application vendors decided not to do so (whether because they thought they knew better or because they didn't bother to read the documentation), then they deserve to get taken out behind the woodshed and thrashed.

Having said that, as a user, if I upgrade to Vista and it makes an application I use frequently not work any more, then I am going to blame it on Vista, not the application.  Even if it really is the app's fault :)  ("It worked perfectly *before* I installed Vista...")

You just can't win.

@Jeremy: I disagree.  It's quite possible and preferable for an API to be written in a language agnostic way.  That doesn't mean you ignore common sources of programming errors, because they're common to many programming languages.  To twist a phrase, a lousy programmer can leak memory in any language.  It's just harder in some than others.  This is especially true in systems such as Win32 that return some resource that need to be freed at a later time, and can't necessarily be just garbage collected.  File handles / descriptors / ports?  Something like that exists almost in almost every OS and if it's just a random small number, well, there ya go.

I must be a real old fart, because I saw this heading and thought immediately of FindFirst, FindNext and FindClose.  In Win 3.1, FindClose did nothing, but you were advised to call it anyway; sure enough, in Win95 FindClose was a dispose, and people who did the Right Thing [TM] were rewarded with fewer memory leaks.

There are a lot of contracts in programming.  The one that DOESN'T exist is the one that says "This is going to be easy, so you don't need to learn anything".  Programmers who think they can coast on gut feeling and googled example code are bad programmers; no doubt about it.

Why not instead design the API such that the programmer cannot fail to use it correctly and still have their program compile?

I think this mindset of requiring the programmer to follow instructions rather than enforcing the rules in the language is primarily a property of the inexpressiveness of the languages under consideration.

In this case for instance, in a slightly richer language, you could have a function WithEnvironmentStrings which takes a function that takes the environment strings to a procedure to be performed with them, and does whatever allocation and deallocation is required with regard to the table, with the procedure performed in between. You can do this in C too, but your users will hate you, because C has no anonymous functions or procedures.

I think this is what Tony Morris was getting at with his remark that the imperative mindset has things all backwards. It's too hard to express the constraints on how APIs are to be used in your language, so you pass the responsibility off to your users, and then get upset when they don't follow your rules. :)

Instead, it's much nicer to handle issues like this using techniques such as garbage collection in the common case of things like memory, or continuation passing style allocation (like the WithEnvironmentStrings example I mentioned above) in more complex situations, but of course you need a language which provides you enough luxuries to express it.

You want Windows programming to *require* the use of a newer and entirely distinct programming language which is not compatible with any known standard?

I think Cale means 'in hindsight, it would have been better that ...' and it's a good idea for the future.

"but of course you need a language which provides you enough luxuries to express it."

You mean: Using a .NET language such as C#?

You're free to do so, as you're free to use the good old simple Win32 API with a good old simple standard portable language such as C.

One solution to prevent API contract violations, for older API, such as Win32, is to validate every input, even in release versions, since bad programmers don't use debug tools. For system resources leaks, it cannot do more than logging the system leak to a clearly visible and accessible file, through a per-process counter for every type of resources. It has performances implications, though.

In my experience documentation is only used for blaming other people.  This article is no exception. :)

APIs may be language-agnostic, but an API in a given language are automatically bound to the language you are using.

I've seen Python and Perl wrappers of very complicated C(++) APIs that do indeed take away the problems of leaking (if we assume you trust the Perl/Python GC). But that doesn't do anything to change the fact that if you use the native C(++) API, you've got to worry about such things.

Demanding a C(++) API that automatically handles these things and makes the programmer not have to think is demanding something other than a C(++) API. Which in the abstract is OK; I'd program .Net before C(++) Win32 in a heartbeat. But that doesn't change a single thing about Raymond's post, which already starts with the assumption that we're in C(++). And if you're programming in C(++) without a constant awareness of memory issues, you've *already lost*, long before Microsoft changed an API in a way they told you about in advance. Lossage that I've seen more times than I can count, so it's hardly theoretical.

of course it is also incredible that the api was designed that way at first. system architects should be able to forsee the future problems of a no-op function. unfortunately all kinds of problems like this can arise in unmanaged code. they are much less likely in managed code.

"of course it is also incredible that the api was designed that way at first."

Who said that the API was designed that way at first?

"However, some people noticed that on Windows NT 4, the Unicode version of the FreeEnvironmentStrings function didn't do anything."

Notice that this says "on Windows NT 4" and "the Unicode version", not "since the function was first introduced, all versions of this function did nothing".

If you program in a native language, you need to know what memory is. No excuses! The sad reality is that there are many native code programmers who don't.

Paraphrasing Anonymous, Matt, Cale, et al:

"Why can't you just design a telepathic and clairvoyant API that automatically fixes all my boneheaded bugs for me?"

What surprises me isn't that some people have a habit of writing poor code.  What surprises me is that they can be so entrenched in that habit that they will loudly proclaim that it's really somebody else's problem.

As a sometimes systems-programmer myself, I'm dismayed that it took until just a couple of comments ago before somebody pointed out that it was ALSO a dumb thing to stick in a do-nothing function call and then assume people would call it by contract.

Uh, did that REALLY seem like a good idea to anybody? By Windows NT 4, had we not all figured out a long time ago that many application developers are not going to do things exactly the way you tell them?

This is MS's error first; the app developer's second. Neither one is right, but MS was more wrong.

[Okay, you have a time machine. What do you do? Do you have the cleanup function do some pointless busy work just so it does "something"? -Raymond]

no, you _always_ create a copy for the application. what if the app decides to use the returned buffer as some kind of working memory an does string operations in it? this is not the least unlikely. future calls of the Get-api will then return the modified result.

"what if the app decides to use the returned buffer as some kind of working memory an does string operations in it?"

Then the app is ignoring the part of the documentation that says:

"Treat this memory as read-only"

Aside from that, the end-to-end principle applies. If you need a copy for applications-specific reasons, make a copy. It's not the operating system's job to write your application for you.

"Then the app is ignoring the part of the documentation that says"

this is constantly happening. after all, this blog is partially about the phenomenon of bad code. as a systems designer you cannot ignore this. i even have to think about it when coworkers are using my code!

"If you need a copy for applications-specific reasons, make a copy. It's not the operating system's job to write your application for you."

of course it is! the operating system provides services to you in order to alleviate the need for implementing them yourself. the reason for choosing a development platform is that one platform might reduce your work more than the other.

The name of the API is "GetEnvironmentStrings." Its job, believe it or not, is to get environment strings and then let the application move on-- not to anticipate whether you need a copy, how many, for how long, from which allocators, etc., etc. All of those are things the application can manage for itself, using other APIs, or no APIs.

what would you have done at the time you created your first win32 window? i believe that you are now capable of using win32 correctly but only because you have gone through every misery unmanaged code has to offer at least once ;-) unfortunately many programmers are, after 10 years of experience, still at the entry level.

an api can be designed to help you and it can be designed without consciousness of usability. GetEnvironmentStrings one is of the latter (although the usability problem is only minor. there are other examples as well).

[Okay, you have a time machine. What do you do? Do you have the cleanup function do some pointless busy work just so it does "something"? -Raymond]

I don't think the implementation is flawed, but if I had a time machine I might take a very defensive approach: Counting resource references. Incrementing a process-wide counter when GetEnvironmentStrings is called and decreasing it when FreeEnvironmentStrings is called. If the count, when a process terminates or exit, is non-zero, this event would be logged.

However, its limit are obvious:

1) It doesn't prevent application from calling FreeEnvironmentStrings on the wrong pointer.

Adding testing logic is possible, but would necessarily GetEnvironmentStrings to copy the string.

(An instance ordinal would be added in the bytes preceding the string... People would obviously mess with it...)

2) Programmers would ignore the logs.

3) It is making the release version of Windows look like a debug version. Just because bad programmers don't use debugging tools, end users would suffer from performances of debugging tools.

@M1EK: So, are you telling us that the average programmer doesn't read the manual but disassemble the Windows code? I cannot believe that!

I would rather expect the bad programmer not to care about resource leaks as far as the memory leaks don't make his program crash in an "out of memory" condition afer 10 minutes, which is not likely to happen with little-used resources such as environment blocks and kernel handles.

"what would you have done at the time you created your first win32 window?"

I remember very well. I carefully read the SDK documentation and took an extremely defensive approach everywhere I wasn't sure... Of course, as I usually do, I freed resources in the reverse order of their allocation. Later, as I better knew the documentation, I relaxed my programming technique.

I never disassemble the Windows code to see what an API does and not just because the EULA forbids it. Most of the time either Raymond explains it, or I look at what Wine does and that often explains things.

If we really want to criticize the API design in any detail, I would also point out that getting the entire environment block is very rarely what you want.  Nor is the format of the environment block very application-friendly to parse, and there are plenty of opportunities for bugs there, too (can you say case sensitivity problems?).

I would rather keep the format and order of the environment block hidden (so you're free to change it -- what if you wanted to change it to a hash table or binary tree to increase performance?), and only provide getenv/setenv APIs.  There is then the question of providing an environment block at process creation, but there, I expect you generally either want to (A) inherit everything or (B) build a new environment from scratch, selectively inheriting using getenv() if necessary.

Ding ding ding for Matt Craighead's answer.

Also, you don't need a time machine to identify that more than one party was to blame here. That doesn't change the solution now, but it does help us possibly avoid future problems of this type.

A programming API that requires that every programmer have read every single line of the manual to avoid dying in flames is not a good API. There was a time I felt differently, of course. I got better.

You don't need to read every line of the manual in this case.

You have a function that returns a pointer (and a non-const one, no less).  People with common sense should ask, "who owns the pointee, and if it's the caller's responsibility to free it, how does the caller do so?".  You don't even need to look at the manual first to know to ask these questions; they're implied by the function signature.

A quick scan through the documentation should answer those questions.

But sadly, I admit that common sense is not nearly as common as it ought to be.

Windows APIs has a lot of magic, can't be sure about anything.

C functions which returns buffers are always tricky, easy to get it wrong both when implementing the api code and the application code.

"const" keyword doesn't compile to anything, and isn't enforced when executing.