0

This is likely basic but it's been a long while since I used this. I have seen examples like each of the following:

IErrorInfo *pError;
HRESULT hrError = ::GetErrorInfo(NULL, &pError);
//more code here            
if (SUCCEEDED(hrError) && pError) {
    //more code here            
    pError->Release();
}

and then elsewhere

IErrorInfo *pError;
HRESULT hrError = ::GetErrorInfo(NULL, &pError);
//more code here            
if (SUCCEEDED(hrError) && pError) {
    //more code here            
}
pError->Release();

Which of these is the proper way to use the Release() here? Does it matter; and if so why?

Mark Schultheiss
  • 32,614
  • 12
  • 69
  • 100
  • They are both wrong. This kind of "don't tell me I have a null pointer bug" coding style is pervasive and doesn't do anything but create "it doesn't work" bug reports. You can't avoid eventually finding the bug in the 2nd snippet. – Hans Passant Feb 04 '16 at 23:45
  • @HansPassant perhaps this the unstated part of what bothered me about the first example, now what would be the better way to do that? – Mark Schultheiss Feb 05 '16 at 12:03
  • Just delete ` && pError` – Hans Passant Feb 05 '16 at 12:06
  • See my answer on https://stackoverflow.com/questions/23212380/memory-leak-on-createerrorinfo-when-analyze-from-debugdiag – gast128 Aug 16 '22 at 16:41

1 Answers1

1

The first usage is correct, although in the "more code here" you must be careful not to throw an exception.

It would be better to use a smart pointer instead of IErrorInfo *, which will automatically call Release() when going out of scope. Then your code will not leak in the case of the "more code here" throwing an exception.

The second one is wrong, because if pError is null or indeterminate, then dereferencing it causes undefined behaviour.

M.M
  • 138,810
  • 21
  • 208
  • 365