Komfortabler Dateimanager mit vielen Funktionen

Analyseergebnisse

By Sven on 17.03.2008 - 16:54 in Entwicklung

Ich hatte euch ja schon über die Möglichkeiten der statischen Codeanalyse berichtet. Mittlerweile habe ich die meisten Module und Anwendungen mit /analyze kompiliert und alle angezeigten Warnungen beseitigt. Das waren gar nicht mal so wenige. Ein Teil war auf meine Bequemlichkeit zurückzuführen, für kleine dynamische Speicherallokationen keine Prüfung auf einen möglichen NULL-Zeiger durchzuführen. Im Normalfall ist es zwar eher unwahrscheinlich, dass kleine Anforderungen nicht befriedigt werden können, aber das ist dem Codeanalyse-Tool völlig egal und es meckert den Fehler zurecht an.

Auch die Freigabe von mit new WCHAR[260] allokiertem Speicher durch delete wurde kritisiert, richtig wäre delete[]. Bei objektlosen Datentypen macht das zwar keinen Unterschied, bei Objekten hingegen (z.B. new CObject[20]) werden bei einem einfachen delete keine Konstruktoren mehr aufgerufen.

Bei der Zeile

if (NULL != pHeaderCtrl && GetKeyState(VK_LBUTTON > 0))

wurde die Warnung

warning C6326: Potenzieller Vergleich einer Konstanten mit einer anderen Konstanten.

ausgegeben. Da liegt das Codeanalyse-Tool vollkommen richtig, den eigentlich sollte es auch

if (NULL != pHeaderCtrl && GetKeyState(VK_LBUTTON) > 0)

heißen. Die folgenden Zeilen sollten prüfen, ob die Variablen vom Typ FILETIME mit Werten ungleich 0 belegt sind. Wenn ja, dann ist die Zeit gesetzt und kann für den Aufruf von SetFileTime verwendet werden.

// Zu setzende Zeiten
BOOL fCreationTime = pCreationTime->dwHighDateTime && pCreationTime->dwHighDateTime ? TRUE : FALSE;
BOOL fLastAccessTime = pLastAccessTime->dwHighDateTime && pLastAccessTime->dwHighDateTime ? TRUE : FALSE;
BOOL fLastWriteTime = pLastWriteTime->dwHighDateTime && pLastWriteTime->dwHighDateTime ? TRUE : FALSE;

Das Codeanalyse-Tool war damit aber nicht einverstanden und meldete

warning C6287: Redundanter Code: Die Teilausdrücke links und rechts sind identisch.

Völlig korrekt, denn eigentlich sollte auch dwLowDateTime auf 0 geprüft werden. Glücklicherweise hat dieser Fehler keine Auswirkungen, denn kaum jemand wird eine Datei auf die ersten sieben Minuten des 01.01.1601 datieren wollen. Aber gut, dass auch solche versteckten Fehler gefunden werden.

Speziell bei diversen Stringformatierungen im printf-Stil spielt das Codeanalyse-Tool seine Stärken aus. Bei diesem Codeschnippsel

CString strMessage;
strMessage.Format(L"Fehler: %s:%s", GetLastError(), L"Fehlermeldung");

würde zum Beispiel die Warnung

warning C6067: Der 2-Parameter im Aufruf von “ATL::CStringT::Format” muss die Adresse der Zeichenfolge sein.

ausgegeben werden. Das Gleiche gilt natürlich auch für alle anderen verwendbaren Datentypen. Das Codeanalyse-Tool erkennt auch, wenn für den Parameter %s bei Unicode-Kompilierung statt eines Unicode-Strings ein Ansi-String übergeben wird. Dank diesem Hinweis konnte ich nun auch endlich klären, warum bei der Schnellansicht von Programmdateien bei den Bound Import Descriptors nur chinesische Schriftzeichen ausgegeben werden.

Bei Aufrufen von CString::FormatMessage kommt das Analysetool aber leider etwas durcheinander. Die einzelnen Parameter im Formatierungsstring werden durch %1%99 gekennzeichnet. Für einen String-Parameter  kann man entweder %1 oder %1!s! schreiben, was das Codeanalyse-Tool aber nicht erkennt. Somit meldet es irrtümlicherweise immer, dass zuviele Parameter übergeben werden.

Auch bei einigen sicheren CRT-Stringfunktionen werden Fehler gemeldet, die keine sind. sscanf_s erfordert zum Beispiel, dass für Stringparameter neben dem Speicher für den String auch dessen Größe übergeben werden muss, damit es zu keinem Pufferüberlauf kommt. Das Codeanalyse-Tool will davon aber nichts wissen und bemängelt unablässig die “falsche” Parameterliste. Hier hilft es nur, die Warnungen vor dem Aufruf der Funktion per #pragma warning(disable: Fehlernummer) temporär zu deaktivieren. Nach der Funktion aber die Wieder-Aktivierung nicht vergessen.

Im nächsten Schritt werde ich jetzt auch meine eigenen Bibliotheken mit den entsprechenden Codeanalyse-Tags versehen. Das wird sich aber vermutlich über mehrere Monate hinziehen, da ich das auch gleich mit einem intensiven Codecheck verbinden werde. Im Laufe der letzten Jahre haben sich schon ein paar Zeilen angesammelt, die man heute sicher ganz anders (und besser) schreiben würde.

Es gibt 3 Kommentare zu diesem Beitrag

Trackback URL | RSS-Feed für Kommentare

  1. hanni sagt:

    viel spass 😀

  2. pat sagt:

    Verbesser mich, aber ich meine mich erinnern zu können das ein delete bei new WCHAR[260] ein undefiniertes verhalten hinterlässt. wäre somit also compilerabhängig waspassiert.

  3. Oliver sagt:

    Keinen Unterschied würde ich bei delete vs. delete[] nicht sagen. Versuche mal einen Debug-Build unterm Debugger auszuführen, wenn du die beiden verwechselst 😉

    @pat: Ich denke da hast du recht. Der VC-Kompiler allerdings stützt dich in jedem Fall auf die CRT malloc/free Implementationen (es sei denn man fummelt dran rum um das zu ändern).

    @Sven: schon mal an PCLINT gedacht? Das erkennt beispielsweise auch interessante Dinge wie Vertauschungen in der Initialisierungsliste eines Konstruktors im Vergleich mit der Reihenfolge in der die Membervariablen deklariert wurden. Kann zu bösen und schwer findbaren Fehlern führen.

    // Oliver

Top