What's wrong with this code - part 18, bonus answer

So the answer to my bonus question was too easy, "hippietim" figured it out in the first comment.  The problem is that the loop:

for (LONG i = 0 ; i < imageCount ; i += 1)
{
hr = images->item(CComVariant(), CComVariant(i), &image);
    if (FAILED(hr))
{

 

Leaks all the "image" object references except the last one. If you're lucky enough to be running a debug version of the runtime library, the code will assert, but that doesn't always happen.

The fix, of course is to move the "image" variable to the correct scope.

for (LONG i = 0 ; i < imageCount ; i += 1)
{
CComPtr<IDispatch> image;
hr = images->item(CComVariant(), CComVariant(i), &image);
    if (FAILED(hr))
{

Other errors pointed out in the comments (mea culpas): Several people (Aaron, Vladimir, Miral) pointed out that instead of exit()ing, I should have used throw hr;, they're right, I was mixing metaphors.

There's one other thing that came up in the comments, it's worth its own post (to increase visibility) so I'll post that tomorrow.

Comments

  • Anonymous
    March 21, 2006
    It wont compile because of unmatched BRACES :) DO i win? :D
  • Anonymous
    March 21, 2006
    The for loop, it really is a badly named while loop construct dating back from the 70s.
  • Anonymous
    March 22, 2006
    Hippie's rule!