Preferred coding style for error handling?

[Mike Hearn]

Hi,

I was wondering if the gurus that hang out here had advice on the best
way to handle errors. At the moment, the most straightforward way seems
to be to write code like this:

DWORD res = 1;
    HKEY key = NULL;

res = RegOpenKeyEx(hCurrent, subkey, 0, KEY_ALL_ACCESS, &key);
    if (res != ERROR_SUCCESS) goto end;

.... do stuff with key here .....

res = 0;
end:
    if (key) RegCloseKey(key);
    return res;

but obviously pretty much every coding guideline ever written says
"don't use goto!". Given the lack of any exception handling in C, and so
no try/finally, is there a better way to deal with this? I've seen a mix
of styles dotted around the wine source, this seems the easiest to read.
Any opionions? Don't want to start a flamewar, but there might be
arguments I've not seen yet.....

thanks -mike

[Dimitrie O. Paun]

On 25 Aug 2003, Mike Hearn wrote:
> but obviously pretty much every coding guideline ever written says
> "don't use goto!".

IMO they are too religious. Yes, avoid goto, but not at the (big) expense
of readability. There are not that many cases where you need goto, but this
is one of them. I find this style a lot more readable than 10000 nested
ifs or what have you. Use good judgement and taste, and if you find that it 
results in cleaner code, go for it.

[Jakob Eriksson]

Mike Hearn wrote:
> but obviously pretty much every coding guideline ever written says
> "don't use goto!". Given the lack of any exception handling in C, and so

I have done a lot of C and oldstyle C++ coding, and I have never felt
the need for a goto.
I find that I can almost always do it by having many returns instead,
all at the top, checking
for failure modes. That way you avoid the nested ifs and you always know
the real businness
is at the bottom of the function.

DWORD res = 1;
    HKEY key = NULL;

res = RegOpenKeyEx(hCurrent, subkey, 0, KEY_ALL_ACCESS, &key);
    if (res != ERROR_SUCCESS) {

return res;
    }

.... do stuff with key here .....

RegCloseKey(key);  /* **) */

return res;

**) In this particular example I assume that if RegOpenKeyEx returns not
ERROR_SUCCESS,
then the key is invalid and need not closing.

When this approach is not working, I usually find that the function I
try to implement is
too big and need to be split up in subfunctions.

regards,
Jakob

[Ian Goldby]

On Wednesday 27 Aug 2003 08:09, Jakob Eriksson wrote:
> I have done a lot of C and oldstyle C++ coding, and I have never felt
> the need for a goto. I find that I can almost always do it by having
> many returns instead.

This is the technique I usually use. The only thing you have to watch is to 
make sure that you tidy up properly just before each return. In extreme 
cases, it's probably better to use goto (especially if undoing a non-existant 
action is benign - unfortunately with malloc/free it's not):

if (!(ptr = malloc(...)))
  return;
if (CreateMutex(...))
{
  free(ptr);
  return;
}
if (CreateEvent(...))
{
  free(ptr);
  DeleteMutex(...);
  return;
}
   etc.

[Dimitrie O. Paun]

On Wed, 27 Aug 2003, Ian Goldby wrote:
> This is the technique I usually use. The only thing you have to watch is to 
> make sure that you tidy up properly just before each return. In extreme 
> cases, it's probably better to use goto (especially if undoing a non-existant 
> action is benign - unfortunately with malloc/free it's not):
>
> if (!(ptr = malloc(...)))
>   return;
> if (CreateMutex(...))
> {
>   free(ptr);
>   return;
> }
> if (CreateEvent(...))
> {
>   free(ptr);
>   DeleteMutex(...);
>   return;
> }
>    etc.

Right -- in most cases (including malloc/free) initializing variables to a
known value (usually 0) solves such problems. If that does not work, you
can have a multi-stage exit, and just goto the right stage. But I agree
with you such cases are not very common. But for the cases where this
makes sense, I'd rather make good use of goto rather than make a complex
case even harder to understand just for the benefit of sticking to the
don't-use-goto dogma.