What's wrong with this code, part 15

  • Article

Work's been pretty hectic lately, that's why so few posts this month, but I ran into a real bug in my code recently that I realized would make a GREAT "What's wrong with this code" post.

 

 HRESULT CNotification::GenerateEvent 
   (     
      PNOTIFICATION_BLOCK NotificationBlock
   ) 
{ 
    HRESULT hr = S_OK; 
    BYTE *buffer = NULL; 
    DWORD bufferSize = sizeof(NOTIFICATION_HEADER) + 
                            NotificationBlock->BlockSize; 
    if (NotificationBlock->BlockSize < sizeof(NOTIFICATION_BLOCK)) 
    { 
        hr = E_INVALIDARG; 
        goto Error; 
    } 
    else 
    { 
        buffer = new BYTE[bufferSize]; 
        if (buffer == NULL) 
        { 
            hr = E_OUTOFMEMORY; 
            goto Error; 
        } 
        PNOTIFICATION_HEADER notificationHeader = (PNOTIFICATION_HEADER)buffer; 
        PNOTIFICATION_BLOCK notificationBuffer; 

        ZeroMemory(buffer, bufferSize); 
        notificationBuffer = (PNOTIFICATION_BLOCK)(notificationHeader + 1); 

        <Fill in the EVENT_TRACE_HEADER within the NOTIFICATION_HEADER structure>

        CopyMemory(notificationBuffer, NotificationBlock, NotificationBlock->BlockSize); 

        hr = HRESULT_FROM_WIN32(TraceEvent(_TraceHandle, (PEVENT_TRACE_HEADER)&notificationHeader._TraceHeader));
        if (hr != S_OK) 
        { 
            goto Error; 
        } 
    } 
Cleanup: 
    delete []buffer; 
    return hr; 

Error: 
    goto Cleanup; 
}

Pretty straightforward, but it's got a REALLY nasty bug in it (I was SO embarrassed when I found it).

 

As always, kudos and mea culpas next post.

Edit1: Fixed typo (mediaBuffer->buffer).  Also fixed NOTIFICATIONBLOCK that should be PNOTIFICATIONBLOCK