| Date: | January 23, 2006 / year-entry #29 |
| Tags: | code |
| Orig Link: | https://blogs.msdn.microsoft.com/oldnewthing/20060123-12/?p=32573 |
| Comments: | 39 |
| Summary: | There are many cases where a callback function is allowed to halt an operation. For example, you might decide to return FALSE to the WM_NCCREATE message to prevent the window from being created, or you might decide to return FALSE to one of the many enumeration callback functions such as the EnumWindowsProc callback. When you... |
There are many cases where a callback function is allowed to halt an operation. For example, you might decide to return Of course, when this happens, the enclosing operation doesn't know why the callback failed; all it knows is that it failed. Consequently, it can't set a meaningful value to be retrieved by the If you want something meaningful to be returned by the This is something that is so obvious I didn't think it needed to be said; it falls into the "because computers aren't psychic (yet)" category of explanation. But apparently it wasn't obvious enough, so now I'm saying it. |
Comments (39)
Comments are closed. |
sooner or later you have to learn that there is no such thing as obvious, I always try to go by this statement, but there was an article I wrote about some web technology and one guy just couldn’t get it to work, I told him all the things I could think of that he could check to make it work, on the end he replied, I found the problem!!!
IIS has to be running!
<<IIS has to be running!>>
"And any electric device works better when plugged and turned on."
I wonder if this (or something similar to this) is what’s behind many of the "operation XXX failed: the operation completed successfully" error messages that I see from time to time. I suppose it might also be that one of the APIs (or internal functions of the program) isn’t doing a SetLastError(), or that the program is doing a SetLastError(0) (maybe because it’s calling another API that does it) before showing or logging the error message.
Anyway, that’s close to my #1 pet peeve of programs: not including enough information in their error message(s) so that I can figure out what went wrong after they fail.
This is something that exceptions would solve (if done properly). Yes, I know exceptions are hard to get right …. but error-passing can be too!
You’ve been in MS so long, Raymond, that you forget… ISV’s don’t have access to the source, they don’t know what all the obvious (but unwritten) rules are. They don’t see the best practices in other parts of the Windows source. They don’t get reviewed by people like yourself who know all the tricks.
You link to the docs for EnumWindowsProc… does it say anything about SetLastError. Nope. And Windows docs and samples have gotten much better in the last 5 years. Imagine the poor folks struggling with apps before that.
That has always been the value of an experienced Windows programmer… not that they know the API; it’s always been pretty basic. It’s that they’ve (painfully) learned all the hidden tricks and gotchas that were "obvious" to MS employees.
Peter: But how could it be any other way? What error code should be returned if a callback indicates that the operation failed?
"
But how could it be any other way? What error code should be returned if a callback indicates that the operation failed?
"
Your question assumes someone implementing WM_NCCREATE or EnumWindowsProc is also considering how to implement CreateWindow or EnumWindows. Do you think that is a reasonable assumption?
Yes I think it’s a reasonable assumption. Software doesn’t consist of magical mind-reading fairies.
Maybe not implementing EnumWindows, but if you’re implementing EnumWindowsProc you should at least read the documentation for EnumWindows:
"If EnumWindowsProc returns zero, the return value is also zero. In this case, the callback function should call SetLastError to obtain a meaningful error code to be returned to the caller of EnumWindows."
Maybe such a doc-section should be added to other similar functions too?
I see this problem all the time with newbies trying to implement a window proc, getting something wrong, end up aborting the creation of thier window, and then complaining that CreateWindow is buggy because it fails for no reason and GetLastError doesn’t return anything meaninful.
Perhaps the logic could go:
(inside CreateWindow…)
SetLastError(ERROR_WNDPROC_RETURNED_FAILURE) // assume generic failure
(eleventy-billion things that need to happen to create a window…)
if (!error) SetLastError(NO_ERROR)
(return from CreateWindow)
I was going to post a comment here about this, but I was so angry about Raymond’s rather rude responses above that I ended up writing an entire diatribe about the Windows API documentation, and put it in my blog. It mentions several of the many API bugs I’ve found, and includes a list of three very exacting bullet-points that, if followed, would make the API documentation a lot more user-friendly and a lot less susceptible to problems like this. (These are not rough fuzzy vague suggestions, either; they’re all of the "document this specific list of things" variety.) Here’s my blog entry, for those who care to read it:
http://www.werkema.com/blog/index.php?op=ViewArticle&articleId=114&blogId=1
And I think I’ve concluded that programmers should be legally prohibited from either designing user interfaces or defining good documentation, because both are exercises in a field in which most programmers are highly unskilled: Human interaction.
Sheesh. Magical mind-reading fairies indeed.
The misunderstanding can arise when someone who is, er, "inexperienced" (read: "not a mind reader him/herself") doesn’t realize that, say, returning FALSE from an enum callback is semantically equivalent with a failure (in the win32 world).
What if some poor chum just finds the window he is looking for and decides to stop enumeration? It won’t occur to him that he caused the error indicated by the windows API. If I had written the API, stopping enumeration by returning FALSE would not be an error. If I had stopped enumeration because I had encountered an error, I wouldn’t need to propogate that back through the windows API–I set the dang error, I ought to know what it is.
If you don’t need to propagate the reason why you stopped enumerating back through the Windows API, then more power to you. In fact, most people do it this way – recording the failure reason in some other place. But if you choose to do it this way, then don’t complain that GetLastError doesn’t return anything interesting – you already decided to return the reason somewhere else – go look in that other place you yourself put it.
I always wondered why there weren’t enough detailed error messages in the debugging output window. With the Windows debug DLLs you would sometimes get very nice detailed messages of why an API call failed, but more often it was a general message (like a dialog creation failed) that shed little light on the problem. I don’t know how many times I exclaimed to myself over the years, "I know you know what I did wrong, why don’t you just tell me?".
Could it also be that we’re not exactly sure what’s going to happen to the global error value? It never even crossed my mind to use SetLastError/GetLastError to pass this kind of application-specific error because I have no idea what Windows might do between the time my callback exits and the caller regains control. I wouldn’t be surprised to find that user32 sets the error value to something indicating general failre, wiping out my specific value. As mentioned above, we don’t have source. We’re in the dark. So, even when I need to capture this information, I always resort to an application-specific mechanism rather than a system supplied one that I cannot trust.
But you see, I might not understand why EnumChildWindows returns a failure. If in the way that I would have written win32, a callback returning false does not indicate a failure, than EnumChildWindows must have encountered some other problem, which I dutifully investigate.
Again, the problem is that I may not know that my callback was the actual source of the error condition, and not something else. Programmers aren’t mind-readers, either.
"If I had written the API, stopping enumeration by returning FALSE would not be an error."
And that’s how it’s designed today. If you don’t call SetLastError, then there is no error code, so there is no error.
"I may not know that my callback was the actual source of the error condition, and not something else."
That’s why most people store the "real" reason for stopping the enumeration out-of-band. That’s my recommendation as well.
I’m going to slightly disagree with Raymond’s recommendation.
Don’t use SetLastError() to propagate out a status. Instead almost all of these APIs take a PVOID context that’s passed through to the callback function. In the actual context struct you should have a place where you can indicate that you chose to end the callback sequence early and it’s because of <x>. (Where <x> may be "I hit error code <Z>" or it might be just that I found what I was looking for and we don’t need to continue scanning.)
Unfortunately not everyone who authored a callback-based interface had the forethought to provide a parameter for a context to be passed to the callback (hey when you’re thinking single threaded, the address space is your context!) in which case, while you /can/ fix this yourself using per-thread data to store the context pointer (as long as your callback is trivial), just setting the preexisting per-thread-context (the win32 last error value) suffices.
We should log a doc bug about this because the fact that the last error value has to be maintained between the callback and the original caller is nonobvious but is in fact part of the interface.
But I never like the easy way. It’s usually fraught with danger.
But there is an error. Or there appears to be, because EnumChildWindows returns zero.
Thankfully, the documentation for EnumChildWindows has been amended (informing the consumer of the API that if s/he returns zero from the callback, so will the enum function), so hopefully this will not actually happen to anyone.
Unfortunately, the method described for WM_NCCREATE doesn’t work for WM_CREATE (when -1 is returned), because Windows seems to reset the last error code after WindowProc returns from WM_DESTROY. Also if FALSE is returned for WM_NCCREATE, Windows still calls WM_NCDESTROY, so the last error can also be set in WM_NCDESTROY.
My favorite error message that I encountered recently was something like,
"Unable to delete [filename]: There is not enough disk space."
This was on a drive with 0 bytes free. At first glance it seems completely contradictory and ridiculous :) It was in fact a manual deletion from Explorer, so there was probably some bookkeeping in the Recycle Bin that required a small amount of disk space.
That’s the thing; ideally, all the API routines that call callbacks would do something like:
SetLastError(ERROR_CALLBACK_FAILED);
if(!Callback(…)) return FALSE;
SetLastError(ERROR_SUCCESS);
…
so that if the callback returns failure without setting the error code then any generic error-handling code watching the enclosing call could at least see "oh, it failed because the callback told it to" — and if it did set an explicit error code, then it’s preserved.
But we can’t assume that, we don’t have the Windows code. For all we know, the SetLastError is immediately before the "return FALSE", so that anything we do is overwritten. Or as part of its cleanup prior to exit, some other API that it calls internally nukes the error code.
Unless we’re explicitly told that error codes get propagated out of callbacks, we have to assume that they’re not, and therefore that there’s no point in calling SetLastError. (Sure, we could try it and see if it works. But if it’s not documented then it’s not part of the spec and may change in a future version of Windows. So we can’t rely on it.)
And I keep forgetting to put a different name in. If I put my normal nick ("Miral") in there then it goes and puts a link in to an MSDN blog. And that’s not my link. Arg.
I think the Windows API has awesome documentation compared to most API’s. Most of the things people complain about are in the tutorial sections, but most people don’t read those, apparently.
No, it’s not Windows’, it’s the Tcl/Tk documentation that sucks. I think Microsoft should give it a good workover. I’m sure they could do a better job than what they got right now.
Let’s face it: all documentation sucks. That’s no reason to stop suggestions for making it suck less, though ;)
And when it comes to documentation, there really is no such thing as "this is too obvious to mention" — if someone reads your docs and ends up thinking the wrong thing, then it’s almost always the docs at fault.
AnotherMatt:
"I think the Windows API has awesome documentation compared to most API’s."
I agree. I have never had any problem finding what I need in the MSDN library. Some of the descriptions can be vague at times (I cannot think of an example), but it usually does not take much effor to figure it out. I would hate to try and teach myself Win32 programming without it (not sure it would even be possible).
Java Document is thousands of times easier to read and navigate then the bloated MSDN. Not only is MSDN loaded with that annoying menu system, but it takes thousands of clicks to get to what you are actually after.
JME anyway.
And back on topic, I, like most others, would never call SetLastError unless I was specifically told to by the doc.
OK, so let me get this straight.
When your WNDPROC gets a WM_NCCREATE message, you should do your thing and then return TRUE (1) to indicate success, or FALSE (0) to indicate failure. In the latter case, you should call SetLastError to deposit an error code for the caller of CreateWindow to pick up after it sees NULL come back from CreateWindow.
On the other hand, the sister message WM_CREATE should be handled by returning 0 (not to be confused with FALSE) on success instead of TRUE, and -1 (where did that come from?) on failure. In case of failure, you can call SetLastError as in the WM_CREATE case, but it won’t do any good, as the error will not survive the trip back to the original caller.
Interesting? Somewhat. So obvious that it doesn’t need to be mentioned? To some, I suppose.
SetLastError() is too easy to be eaten by other operations. After you call SetLastError(), maybe you want to do some error logging stuff. Right there your error code is eaten.
Or maybe someone else want to do the error logging stuff.
You have to copy and restore the error before and after the error logging stuff. Many people did not realize it.
Exceptions may help. But exceptions are too darn hard to get right.
Mike’s suggestion of passing a context object is another alternative. But then it is so not typical.
Make no mistake, error reporting is not a trivial problem.
Yeah, I think error handling should documented better in MSDN. It’s not obvious at all when you should call SetLastError(). In MSDN (July 200) it says:
"This function is intended primarily for dynamic-link libraries (DLL). Calling this function after an error occurs lets the DLL emulate the behavior of the Win32 API."
That’s totally misleading. It should say that SetLastError() is solely for the application programmer and that the Win32 API doesn’t use the value at all. There’s no way to know what the exact purpose of SetLastError() is. It could trigger some error handling somewhere within the Win32 API. Who knows?
I need a SetLastError() that really guarantees what it says. After that, no more errors.
"This is something that exceptions would solve (if done properly). Yes, I know exceptions are hard to get right …. but error-passing can be too!"
I doubt you can let an exception escape from a callback function.
Because it could do memory leaks if the enumeration function has no exception handling and allocates resources. And even if the function is specifically designed to support exceptions, it would only work with a specific implementation of exception handling (probably Visual C++ exception handling system).
The best you can hope, is an absence of resource allocation in the enumeration function.
So, exceptions are not good for that (you must catch all exceptions you throw in your callback function in the callback function itself).
Except when it is specifically documented, you cannot rely on SetLastError behavior as Ring Zero and others noticed.
The only general solution is hand-written systems : setting your own TLS data, or setting fields in a context object passed to the callback function (what I generally do).
If the program is monothreaded, you may also pass data in global/static data (It is a bad idea, but some C programmers do it this way).
IIRC Windows Developer Network (or was it Journal) used to publish in the magazine a set of annotatations that could be added to the standard SDK help which added all the bits that were never documented…
Sorry this is almost completely off topic, but comments are closed on the most relevant posting (two years ago) and it touches on this part of it:
> This is something that is so obvious I
> didn’t think it needed to be said; it falls
> into the "because computers aren’t psychic
> (yet)" category of explanation.
MSDN readers aren’t psychic yet (except for one famous reader ^_^) and some things sure aren’t obvious to ordinary readers. Here’s something which not only tripped up me, it tripped up an MVP, and no one got it right until I spent a day doing Google searches and figuring out the code I was reading.
Take a look at
http://msdn.microsoft.com/library/default.asp?url=/library/en-us/winui/winui/windowsuserinterface/resources/introductiontoresources/resourcereference/resourcefunctions/findresource.asp
and
http://msdn.microsoft.com/library/default.asp?url=/library/en-us/winui/winui/WindowsUserInterface/Resources/IntroductiontoResources/ResourceReference/resourcetypes.asp
Does this make it obvious that this call
FindResource(GetModuleHandle(NULL), MAKEINTRESOURCE(my_string_id), RT_STRING)
isn’t going to find the resource for my_string_id? MSDN doesn’t say it. Even Mr. Chen’s blog entry two years ago on string resources didn’t say that FindResource wasn’t going to find it. Where does the obviousness come from?
Now let’s look at something that the MSDN page on FindResource does say:
> To use a resource immediately, an
> application should use one of the following
> resource-specific functions to find and load
> the resources in one call.
> […]
> LoadString Loads a string-table entry.
In one call eh? Well sure, if the programmer’s psychic powers provided a sufficient buffer length at the time of writing the code. Even MFC’s internals can’t do it in one call, they run a loop growing the buffer and calling LoadString until they think they got the entire string. Well, MFC’s programmers weren’t as psychic as the rest of us have to be. Obviously.
Now sure, I "should" submit these remarks to the contact address at the bottom of the MSDN page. The last time I did that, Microsoft replied saying that my comments were a request for technical support and Microsoft told me how to contact their technical support (yet another famous story). So sorry, I’m sharing this tangent with the rest of us poor psychics.
Have you tried SizeofResource(), which is listed at the bottom of that first page?
2. Some other’s code is executing, possibly calling SetLastError()
3. My code is calling GetLastError()
When you call other code out of your control, then you can NEVER trust previously set error values.
How do I know that the code in EnumWindowsProc/EnumWindows doesn’t call SetLastError() after my callback returned? How can I be 100% sure?
Reading docs. Nope, not mentioned at all.
Reading the source. Nope, it’s not open.
The documentation doesn’t mention it, thus it’s undefined and can be changed tomorrow.
It’s stupid to rely on non documented features, never use SetLastError() in the callback!
There are no documented methods for doing certain common operations. For example, there is no documented way to use GetOpenFileName to ask the user reliably for multiple files. OFN_ALLOWMULTISELECT won’t work because it requires the programmer to allocate telepathically enough memory for the filenames the user is going to select. Using a hook procedure won’t work because there is no documented way for a hook procedure to retrieve the list of selected filenames, or even the length of such a list.
Friday, January 27, 2006 1:38 AM by AnotherMatt
> Have you tried SizeofResource(), which is
> listed at the bottom of that first page?
Have you noticed that one of the parameters to SizeofResource has to be a handle that was returned by FindResource? Did you notice that it would be so hard (or maybe impossible if sticking to documented features) to use FindResource to get a handle to a string resource, that even MFC’s internals don’t use it? They run a loop growing a buffer and calling LoadString until they think they got the entire string? The algorithm is efficient, just coded for … oops, wrong thread.
Anyway, yes I sure did try it before posting. Did you?
If your callback failed because it called another function that failed, you don’t. I.e. If you called RegOpenKeyExW, and it failed with insufficient privileges, you don’t have to call SetLastError(5) again.
However, if it fails with a code 8, you might need to SetLastError(5) too. Because if you open the key without KEY_QUERY_VALUE, XP SP2 will return 8 instead of 5.
Undocumented, of course. Not having source doesn’t help either.