Kategorie: Programmieren

Wieder mal ein Beispiel für einen versteckten Bug.
Nachfolgender Code sieht ganz unscheinbar aus, aber er birgt eine Falle mit sich, die in der Implementierung der Klasse verborgen ist:

void CImageButton::OnSysColorChange()
{
   // Farbe hat geändert
   CButton::OnSysColorChange();
   // Bitmap neu laden
   LoadBitmap(m_strResource.IsEmpty() ? m_lpszResource : m_strResource);
}

LoadBitmap ist eine Funktion, die die Bitmap neu lädt und entsprechend der eingestellten Systemfarben einige Farben aktualisiet. Also das bekannte Verhalten von Toolbars. LoadBitmap nimmt einen LPCTSTR und damit kann die CImageButton Klasse entweder mit einem Ressource String arbeiten (der evtl. mit MAKEINTRESOURCE eine ID ist und kein echter Zeiger), oder eben einem Namen zu einer Bitmap Ressource.

Damit bei einem Wechsel der Systemfarben, die Bitmap neu laden werden kann, merkt es sich die Resource.  In der Funktion findet hier eine Konvertierung des alten CString Wertes in einen LPCTSTR statt, mit dem eingebauten Konvertierungs-Operator.

Das Problem in der Implentierung dieser Klasse war aber, dass LoadBitmap sich den neuen Ressourcennamen merken soll aber zuvor eine interne Clear Funktion aufruft, die die bestehende Bitmap und andere Klassendaten freigibt, bevor m_strResource oder m_lpszResource, neu gesetzt werden:

void CImageButton::Clear()
{
  ...
  // Clear old infos
  m_lpszResource = NULL;
  m_strResource.Empty();
  ...
}

bool CImageButton::LoadBitmap(LPCTSTR pszResource)
{
   Clear();
...
   if (ISINTRESOURCE(pszResource))
...
   else
   {
       m_strResource = pszResource;
...
   }
...
}

Das führt aber nun zu folgendem Problem: Clear löscht m_strResource und damit wird der übergebene Zeiger an LoadBitmap ungültig, mit der Folge, dass die ürsprüngliche Bitmap nicht mehr gefunden wird, wenn in der Zwischenzeit der Heap auf dem dieser String lag neu verwendet wurde. In der Release Version trat dieser Fehler selten auf. In der Debug Version war der Fehler sofort nachvollziehbar, denn hier wird der Heap bei der Freigabe auf einen einhaltlichen Wert zurückgesetzt.

Die Lösung ist einfach: Es ist nötig den alten Inhalt zu kopieren um einen gültigen Zeiger zu behalten.

void CImageButton::OnSysColorChange()
{
  // System colors changed
  CButton::OnSysColorChange();
  // We need to protect the resource name, because LoadItem calls
  // Clear and this might empty m_sResource. And this causes
  // the string object on the stack to get deleted and the pointer
  // points into nowhere land. So we use a copy of the string here.
  CString strRessource(m_strResource);
  LoadBitmap(strRessource.IsEmpty() ? m_lpszResource : strRessource);
}