20

Windows APIs uses GetLastError() mechanism to retrieve information about an error or failure. I am considering the same mechanism to handle errors as I am writing APIs for a proprietary module. My question is that is it better for API to return the error code directly instead? Does GetLastError() has any particular advantage? Consider the simple Win32 API example below:

HANDLE hFile = CreateFile(sFile,
    GENERIC_WRITE, FILE_SHARE_READ,
    NULL, CREATE_NEW, FILE_ATTRIBUTE_NORMAL, NULL);

if (hFile == INVALID_HANDLE_VALUE)
{
    DWORD lrc = GetLastError();

    if (lrc == ERROR_FILE_EXISTS)
    {
          // msg box and so on
    }
}

As I was writing my own APIs I realized GetLastError() mechanism means that CreateFile() must set the last error code at all exit points. This can be a little error prone if there are many exit points and one of them maybe missed. Dumb question but is this how it is done or there is some kind of design pattern for it?

The alternative would be to provide an extra parameter to the function which can fill in the error code directly so a separate call to GetLastError() will not be needed. Yet another approach can be as below. I will stick with the above Win32 API which is good example to analyzer this. Here I am changing the format to this (hypothetically).

result =  CreateFile(hFile, sFile,
    GENERIC_WRITE, FILE_SHARE_READ,
    NULL, CREATE_NEW, FILE_ATTRIBUTE_NORMAL, NULL);

if (result == SUCCESS)
{
   // hFile has correct value, process it
}
else if (result == FILE_ALREADY_EXIT )
{
   // display message accordingly
  return;
}
else if ( result == INVALID_PATH )
{
   // display message accordingly.
  return;
}

My ultimate question is what is the preferred way to return error code from an API or even just a function since they both are the same?

Mysticial
  • 464,885
  • 45
  • 335
  • 332
zar
  • 11,361
  • 14
  • 96
  • 178
  • A `switch` statement seems more appropriate in this case. – Mysticial Feb 01 '12 at 17:06
  • 8
    Why not use exceptions? – GManNickG Feb 01 '12 at 17:09
  • From what I understand, you are asking whether error handling through return codes is a good idea. Well, I prefer exceptions, they make error handling much more comfortable. Check [this question](http://stackoverflow.com/q/1388335/1168156). – LihO Feb 01 '12 at 17:13
  • @GMan See my comments to StilesCrises reply about exceptions. – zar Feb 01 '12 at 17:40
  • 3
    "As I was writing my own APIs I realized GetLastError() mechanism means that CreateFile() must set the last error code at all exit points." Not so. you should only check `GetLastError` if you've been told (via the return value) that it's actually been set. All other times it will be undefined and whatever it was last set to. – Deanna Feb 01 '12 at 19:13
  • @Deanna If I were to use GetLastError() I would at least make sure I use it consistently. That means the API will reset it when entering and set the value accordingly when it exits. If an API doesn't use it at all than it doesn't have to touch it. – zar Feb 01 '12 at 19:41
  • 1
    @Deanna: A nice example of why a `GetLastError`-style error mechanism is a pain to use - it's easy to be inconsistent. – Frerich Raabe Feb 01 '12 at 19:58
  • GetLastError() sucks if for nothing else than making the caller write more code. It is much cleaner to just return the damn error code directly. – Luke Feb 01 '12 at 21:02
  • @GManNickG just appending to my earlier comment, the library will be user by third party users that's why I am not using exceptions. – zar Jan 11 '13 at 14:51

7 Answers7

21

Overall, it's a bad design. This is not specific to Windows' GetLastError function, Unix systems have the same concept with a global errno variable. It's a because it's an output of the function which is implicit. This has a few nasty consequences:

  1. Two functions being executed at the same time (in different threads) may overwrite the global error code. So you may need to have a per-thread error code. As pointed out by various comments to this answer, this is exactly what GetLastError and errno do - and if you consider using a global error code for your API then you'll need to do the same in case your API should be usable from multiple threads.

  2. Two nested function calls may throw away error codes if the outer function overwrites an error code set by the inner.

  3. It's very easy to ignore the error code. In fact, it's harder to actually remember that it's there because not every function uses it.

  4. It's easy to forget setting it when you implement a function yourself. There may be many different code paths, and if you don't pay attention one of them may allow the control flow to escape without setting the global error code correctly.

Usually, error conditions are exceptional. They don't happen very often, but they can. A configuration file you need may not be readable - but most of the time it is. For such exceptional errors, you should consider using C++ exceptions. Any C++ book worth it's salt will give a list of reasons why exceptions in any language (not just C++) are good, but there's one important thing to consider before getting all excited:

Exceptions unroll the stack.

This means that when you have a function which yields an exception, it gets propagated to all the callers (until it's caught by someone, possible the C runtime system). This in turn has a few consequences:

  1. All caller code needs to be aware of the presence of exceptions, so all code which acquires resources must be able to release them even in the face of exceptions (in C++, the 'RAII' technique is usually used to tackle them).

  2. Event loop systems usually don't allow exceptions to escape event handlers. There's no good concept of dealing with them in this case.

  3. Programs dealing with callbacks (plain function pointers for instance, or even the 'signal & slot' system used by the Qt library) usually don't expect that a called function (a slot) can yield an exception, so they don't bother trying to catch it.

The bottom line is: use exceptions if you know what they are doing. Since you seem to be rather new to the topic, rather stick to return codes of functions for now but keep in mind that this is not a good technique in general. Don't go for a global error variable/function in either case.

Frerich Raabe
  • 90,689
  • 19
  • 115
  • 207
  • 3
    `errno` is designed so that it is a macro which expands to some thread local error code. Overall you are right though. – Alexandre C. Feb 01 '12 at 17:16
  • 7
    Windows the same - 'GetLastError' returns a per-thread value. If it did not, it would be unuseable. – Martin James Feb 01 '12 at 17:32
  • 6
    Many of your criticisms of GetLastError are just bogus. It's thread local. Nested functions? You have to check for errors on every API call, nesting is irrelevant. It's very easy to ignore the error code? An API that targets languages without exceptions can't do much else other than use error codes. – David Heffernan Feb 01 '12 at 18:43
  • 3
    @DavidHeffernan: You write 'have to check for errors on every API call' - but not every API call sets the last error! Consider the Windows socket API. This is a poor design, no matter what the language is. It would have been better if it was part of the return value - that way, at least the input/output of each function was clearly visible. The whole *point* of a design is to make correct things easy and incorrect things hard to do. A global value isn't exactly good at that. – Frerich Raabe Feb 01 '12 at 19:50
  • @AlanStokes, @Martin James: I know that `GetLastError` as well as `errno` use some sort of TLS. But when writing the answer, I was thinking about considerations when considering the same sort of design (a global error value, not necessarily the same used by `GetLastError`/`errno`) for your own API. I now see that the OP explicitely asked about whether using `SetLastError` for his own API is a good idea; I was talking about the design, not about a particular implementation of it. I now extended the answer a bit to be explicit about this. – Frerich Raabe Feb 01 '12 at 19:53
  • 1
    I did not say call GetLastError for every API call. I said check for errors. And that's potentially different for each API. Plenty of scope for criticising win32 error handling but not all of your criticisms are valid. MS tried to do better with COM and largely they succeeded. – David Heffernan Feb 01 '12 at 20:22
  • 1
    @DavidHeffernan: I agree that the consistent use of `HRESULT` return values throughout the COM API (together with the `SUCCEEDED` and `FAILED` macros) were an improvement. However, you seem to miss my point: I don't criticize `GetLastError` in particular, I criticize an API design which uses a global error code to signal errors in general instead of, say, using return codes. Or whatever the language (C++ in this case) supports in addition to that. – Frerich Raabe Feb 01 '12 at 21:25
  • I agree with that comment 100% but I'm not sure you said it that clearly in your answer. – David Heffernan Feb 01 '12 at 22:01
  • 2
    "Usually, error conditions are exceptional". I beleive that the most useful property of reporting failures using exceptions is that functions *provide success as a post-condition*. If the code immediately after the method executes, you know the method has done its job. No need for verbose and complicated defensive checks for nulls, values out of range, empty buffers and such like. – Raedwald Feb 03 '12 at 13:04
7

The GetLastError pattern is by far the most prone to error and the least preferred.

Returning a status code enum is a better choice by far.

Another option which you did not mention, but is quite popular, would be to throw exceptions for the failure cases. This requires very careful coding if you want to do it right (and not leak resources or leave objects in half-set-up states) but leads to very elegant-looking code, where all the core logic is in one place and the error handling is neatly separated out.

StilesCrisis
  • 15,972
  • 4
  • 39
  • 62
  • 3
    If you use "normal" C++ techniques (ie. RAII and perusing the standard library), exceptions are easy to use. I agree though with the implied fact that most of the difficulties of C++ come from the need of writing exception safe code. – Alexandre C. Feb 01 '12 at 17:18
  • 1
    I am not too sure if a external library (in my case a dll) should throw exceptions to the outside world. First the library API's are suppose to be generic preferably C style so anybody can use it but even if it is C++, by using exceptions I will be forcing the client application to use/handle those exceptions as well which (I think) is not very common (and more advance work for user of my library). I think exceptions can be used inside my library but not exposed outside it? That leaves us with my original question how to return the error. – zar Feb 01 '12 at 17:36
  • 1
    If you're writing a library to be used by external users, exceptions would be poor form. Many places shun exception handling (including where I work!) and they could reject your library entirely on that basis alone. I wasn't suggesting that exceptions are right for everybody, but a discussion of modern error handling wouldn't be complete without a mention. – StilesCrisis Feb 01 '12 at 21:29
3

I think GetLastError is a relic from the days before multi-threading. I don't think that pattern should be used any more except in cases where errors are extraordinarily rare. The problem is that the error code has to be per-thread.

The other irritation with GetLastError is that it requires two levels of testing. You first have to check the return code to see if it indicates an error and then you have to call GetLastError to get the error. This means you have to do one of two things, neither particularly elegant:

1) You can return a boolean indicating success or failure. But then, why not just return the error code with zero for success?

2) You can have a different return value test for each function based on a value that is illegal as its primary return value. But then what of functions where any return value is legal? And this is a very error-prone design pattern. (Zero is the only illegal value for some functions, so you return zero for error in that case. But where zero is legal, you may need to use -1 or some such. It's easy to get this test wrong.)

David Schwartz
  • 179,497
  • 17
  • 214
  • 278
  • 1
    [GetLastError is threadsafe](http://msdn.microsoft.com/en-us/library/windows/desktop/ms679360(v=vs.85).aspx). – Mooing Duck Feb 01 '12 at 17:57
  • @MooingDuck Of course, the error code is stored as thread-specific data, making every access to it that much more awkward and expensive. (The same thing happened in the UNIX world. The `errno` variable predates threading and so had to be made thread-specific. A more sensible approach is used in newer APIs like the pthreads API.) – David Schwartz Feb 01 '12 at 18:11
  • 1
    Thread local storage access has 0 overhead on windows,linux and solaris as it is just relative to the FS segment register instead of DS or SS. And it's not more awkward the global variable access. – Lothar Feb 02 '12 at 16:37
3

I have to say, I think the global error handler style (with proper thread-local storage) is the most realistically applicable when exception-handling cannot be used. This is not an optimal solution for sure, but I think if you are living in my world (a world of lazy developers who don't check for error status as often as they should), it's the most practical.

Rationale: developers just tend to not check error return values as often as they should. How many examples can we point to in real world projects where a function returned some error status only for the caller to ignore them? Or how many times have we seen a function that wasn't even correctly returning error status even though it was, say, allocating memory (something which can fail)? I've seen too many examples like these, and going back and fixing them can sometimes even require massive design or refactoring changes through the codebase.

The global error handler is a lot more forgiving in this respect:

  • If a function failed to return a boolean or some ErrorStatus type to indicate failure, we don't have to modify its signature or return type to indicate failure and change the client code all over the application. We can just modify its implementation to set a global error status. Granted, we still have to add the checks on the client side, but if we miss an error immediately at a call site, there's still opportunity to catch it later.

  • If a client fails to check the error status, we can still catch the error later. Granted, the error may be overwritten by subsequent errors, but we still have an opportunity to see that an error occurred at some point whereas calling code that simply ignored error return values at the call site would never allow the error to be noticed later.

While being a sub-optimal solution, if exception-handling can't be used and we're working with a team of code monkeys who have a terrible habit of ignoring error return values, this is the most practical solution as far as I see.

Of course, exception-handling with proper exception-safety (RAII) is by far the superior method here, but sometimes exception-handling cannot be used (ex: we should not be throwing out of module boundaries). While a global error handler like the Win API's GetLastError or OpenGL's glGetError sounds like an inferior solution from a strict engineering standpoint, it's a lot more forgiving to retrofit into a system than to start making everything return some error code and start forcing everything calling those functions to check for them.

If this pattern is applied, however, one must take careful note to ensure it can work properly with multiple threads, and without significant performance penalties. I actually had to design my own thread-local storage system to do this, but our system predominantly uses exception-handling and only this global error handler to translate errors across module boundaries into exceptions.

All in all, exception-handling is the way to go, but if this is not possible for some reason, I have to disagree with the majority of the answers here and suggest something like GetLastError for larger, less disciplined teams (I'd say return errors through the call stack for smaller, more disciplined ones) on the basis that if a returned error status is ignored, this allows us to at least notice an error later, and it allows us to retrofit error-handling into a function that wasn't properly designed to return errors by simply modifying its implementation without modifying the interface.

stinky472
  • 6,737
  • 28
  • 27
  • "If a client fails to check the error status, we can still catch the error later." That's only true if every single function (whether it returns an error using this method or not) is required to preserve the error status across a successful invocation. So far as I know, *nobody* does this. (For example, `printf` is permitted to modify the stored error code.) A huge downside to this approach is that you cannot easily check the error code later. – David Schwartz Feb 01 '12 at 18:15
  • We can't either with approaches based on returning error values. The world I come from is one where people often write f() instead of something like if (f()) ... or ErrorStatus status = f(); if (status != success) ... When a failure is made to check the error at the call site here, the error is hopelessly lost. We cannot find out ten lines of code later that an error occurred: we're clueless and in the dark. If we use libraries like OpenGL, people rarely check for errors after every gl call. But we can find out later that a GL error occurred, and eventually to the point of the call site. – stinky472 Feb 01 '12 at 21:08
  • That's what I mean. With something like GL's method, you can at least find out somewhere later down the line that an error occurred even if people aren't checking after every single GL call. The failure to do through error checking does not require that we go back and rewrite everything to start detecting the source of the error, which is kind of the compromising (admittedly sub-optimal) approach I would prefer, as it is more forgiving in that sense. Of course, exception-handling all the way when possible. – stinky472 Feb 01 '12 at 21:10
  • Continuing, let us assume that printf overwrote an original error. Let's say our lazy, sloppy programmer checked neither the original error nor the printf error. At least I can go in at some level and detect the last error, that is the printf error, and start getting to the bottom of things. At least the lazy programming did not obscure all errors, as it would in the case with error return codes that have been ignored. The difference is that the last error still remains for us to detect, even if it is overwritten, we at least caught one error too late than none. – stinky472 Feb 01 '12 at 21:11
  • It's nice to see pro and cons of an approach, thanks for your insight, really appreciate it. When I was writing APIs my gut feeling was that I didn't feel like using GetLastError() approach 'cz it is somewhat 'scattered' and didn't seem modular. – zar Feb 02 '12 at 14:40
  • +1 for realizing right away that I can't use exceptions as I was talking APIs. – zar Feb 02 '12 at 14:54
3

If your API is in a DLL and you wish to support clients that use a different compiler then you then you cannot use exceptions. There is no binary interface standard for exceptions.

So you pretty much have to use error codes. But don't model the system using GetLastError as your exemplar. If you want a good example of how to return error codes look at COM. Every function returns an HRESULT. This allows callers to write concise code that can convert COM error codes into native exceptions. Like this:

Check(pIntf->DoSomething());

where Check() is a function, written by you, that receives an HRESULT as its single parameter and raises an exception if the HRESULT indicates failure. It is the fact that the return value of the function indicates status that allows this more concise coding. Imagine the alternative of returning the status via a parameter:

pIntf->DoSomething(&status);
Check(status);

Or, even worse, the way it is done in Win32:

if (!pIntf->DoSomething())
    Check(GetLastError());

On the other hand, if you are prepared to dictate that all clients use the same compiler as you, or you deliver the library as source, then use exceptions.

David Heffernan
  • 601,492
  • 42
  • 1,072
  • 1,490
  • On windows there is a standard - only unix is again lightyears behind. The SEH is compatible between C and C++ and every compiler that calls itself windows compatible must support SEH in one way or the other as essential windows functions also raise SEH exceptions. – Lothar Feb 02 '12 at 16:35
  • @Lothar: Sorry to wake up an old question, but I have to disagree with the "once again unix is lightyears behind" part. For one thing, standard C doesn't even *have* SEH. That means that it is a compiler extension. And on Linux, binary compatibility isen't even much of an issue. Almost everything is distributed as binary and source. And just a note: We all know what happened last time Microsoft ignored standards. IE. – Linuxios Jul 06 '12 at 18:10
  • @Linuxios I agree. SEH is a red herring here. Standard Windows APIs use GetLastError or HRESULT. Not SEH. – David Heffernan Jul 06 '12 at 18:25
  • @DavidHeffernan: Exactly. While MFC, or COM, or OLE or something more C++ based might, Win32 most certainly doesn't. – Linuxios Jul 06 '12 at 18:29
1

Exception handling in unmanaged code is not recommended. Dealing with memory leaks without exceptions is a big issue, with exception it becomes nightmare.

Thread local variable for error code is not so bad idea, but as some of the other people said it is a bit error prone.

I personally prefer every method to return an error code. This creates inconvenience for functional methods because instead of:

int a = foo();

you will need to write:

int a;
HANDLE_ERROR(foo(a));

Here HANDLE_ERROR could be a macro that checks the code returned from foo and if it is an error propagates it up (returning it).

If you prepare a good set of macros to handle different situations writhing code with good error handling without exception handling could became possible.

Now when your project start growing you will notice that a call stack information for the error is very important. You could extend your macros to store the call stack info in a thread local storage variable. That is very useful.

Then you will notice that even the call stack is not enough. In many cases an error code for "File not found" at line that say fopen(path, ...); does not give you enough information to find out what is the problem. Which is the file that is not found. At this point you could extend your macros to be able to store massages as well. And then you could provide the actual path of file that was not found.

The question is why bother all of this you could do with exceptions. Several reasons:

  1. Again, Exception handling in unmanaged code is hard to do right
  2. The macro based code (if done write) happens to be smaller and faster than the code needed for exception handling
  3. It is way more flexible. You could enable disable features.

In the project that I am working at the moment I implemented such error handling. It took me 2 days to put in a level to be ready to start using it. And for about a year now I probably spend about 2 weeks total time of maintaining and adding features to it.

gsf
  • 6,612
  • 7
  • 35
  • 64
0

You should also consider a object/structure based error code variable. Like the stdio C library is doing it for FILE streams.

On some of my io objects for example, i just skip all further operations when the error state is set so that the user is fine just when checking the error once after a sequence of operations.

This pattern allows you to finetune the error handling scheme much better.

One of the bad designs of C/C++ comes to full light here when comparing it for example with googles GO language. The return of just one value from a function. GO does not use exceptions instead it always returns two values, the result and the error code.

There is a minor group of people who think that exceptions are most of the time bad and misused because errors are not exceptions but something you have to expect. And it hasn't proved that software becames more reliable and easier. Especially in C++ where the only way to program nowadays is RIIA techniques.

Lothar
  • 12,537
  • 6
  • 72
  • 121