Für diejenigen, die Detektiv spielen möchten: Finden Sie den Fehler in der Funktion von Midnight Commander

Wir laden Sie ein, zu versuchen, einen Fehler in einer sehr einfachen Funktion aus dem GNU Midnight Commander-Projekt zu finden. Warum? Einfach so. Es ist lustig und interessant. Obwohl nein, haben wir gelogen. Wir möchten noch einmal einen Fehler demonstrieren, den eine Person bei der Codeüberprüfung nur schwer findet, den statischen Code-Analysator PVS-Studio jedoch leicht findet.

Kürzlich wurde uns ein Brief geschickt, in dem wir gefragt wurden, warum der Analysator eine Warnung für die EatWhitespace- Funktion generiert, deren Code unten angegeben ist. Eigentlich ist die Frage nicht so einfach. Versuchen Sie selbst herauszufinden, was mit diesem Code nicht stimmt.

static int EatWhitespace (FILE * InFile) /* ----------------------------------------------------------------------- ** * Scan past whitespace (see ctype(3C)) and return the first non-whitespace * character, or newline, or EOF. * * Input: InFile - Input source. * * Output: The next non-whitespace character in the input stream. * * Notes: Because the config files use a line-oriented grammar, we * explicitly exclude the newline character from the list of * whitespace characters. * - Note that both EOF (-1) and the nul character ('\0') are * considered end-of-file markers. * * ----------------------------------------------------------------------- ** */ { int c; for (c = getc (InFile); isspace (c) && ('\n' != c); c = getc (InFile)) ; return (c); } /* EatWhitespace */ 

Wie Sie sehen können, ist die EatWhitespace- Funktion sehr klein. Sogar ein Kommentar zu einer Funktion nimmt mehr Platz ein als der Hauptteil der Funktion selbst :). Nun ein paar Details.

Beschreibung der Getc- Funktion:

 int getc ( FILE * stream ); 

Die Funktion gibt das Zeichen zurück, auf das der interne Indikator für die Dateiposition des angegebenen Streams zeigt. Dann geht die Anzeige zum nächsten Zeichen. Wenn das Ende der Datei zum Zeitpunkt des Aufrufs des Streams erreicht ist, gibt die Funktion EOF zurück und setzt das Kennzeichen für das Dateiende für diesen Stream. Wenn ein Lesefehler auftritt, gibt die Funktion einen EOF-Wert zurück und setzt einen Fehlerindikator für den angegebenen Stream (Fehler).

Beschreibung der isspace- Funktion:

 int isspace( int ch ); 

Die Funktion prüft, ob das Zeichen gemäß der Klassifizierung des aktuellen Gebietsschemas ein Leerzeichen ist. Im Standardgebietsschema sind die folgenden Zeichen Leerzeichen:

• Leerzeichen (0x20, ``);
• Seitenwechsel (0x0c, '\ f');
• Zeilenvorschub LF (0x0a, '\ n');
• Wagenrücklauf CR (0x0d, '\ r');
• horizontale Lasche (0x09, '\ t');
• vertikale Registerkarte (0x0b, '\ v').

Rückgabewert Wert ungleich Null, wenn das Zeichen ein Leerzeichen ist, andernfalls Null.

Die EatWhitespace- Funktion sollte alle Zeichen überspringen, die als Leerzeichen gelten, mit Ausnahme des Zeilenvorschubs '\ n'. Ein weiterer Grund für das Stoppen des Lesens aus einer Datei kann darin bestehen, das Ende der Datei (EOF) zu erreichen.

Und jetzt, da Sie das alles wissen, versuchen Sie, einen Fehler zu finden!

Fügen Sie ein paar wartende Einhörner hinzu, um zu verhindern, dass der Leser versehentlich nicht sofort auf die Antwort schaut.

Abbildung 1. Zeit, nach einem Fehler zu suchen. Einhörner werden warten.

Sie sehen den Fehler immer noch nicht?

Die Sache ist, dass wir Leser über isspace getäuscht haben . Ha ha! Dies ist überhaupt keine Standardfunktion, sondern ein hausgemachtes Makro. Ja, wir sind schuldlos und haben Sie verwirrt.

Abbildung 2. Ein Einhorn vermittelt dem Leser einen falschen Eindruck davon, was isspace ist .

Eigentlich sind wir und unser Einhorn natürlich nicht schuld. Die Autoren des GNU Midnight Commander-Projekts trugen zur Verwirrung bei, indem sie beschlossen, eine eigene isspace- Implementierung in der Datei charset.h zu erstellen:

 #ifdef isspace #undef isspace #endif .... #define isspace(c) ((c)==' ' || (c) == '\t') 

Durch das Erstellen eines solchen Makros haben einige Entwickler andere Entwickler verwirrt. Der Code wird unter der Annahme geschrieben, dass isspace eine Standardfunktion ist, die Wagenrückläufe (0x0d, '\ r') als eines der Leerzeichen betrachtet.

Das implementierte Makro berücksichtigt nur Leerzeichen und Tabulatoren als Leerzeichen. Lassen Sie uns das Makro ersetzen und sehen, was passiert.

 for (c = getc (InFile); ((c)==' ' || (c) == '\t') && ('\n' != c); c = getc (InFile)) 

Der Unterausdruck ('\ n'! = C) ist redundant (redundant), da sein Ergebnis immer wahr ist. Der PVS-Studio-Analysator warnt davor und gibt eine Warnung aus:

V560 Ein Teil des bedingten Ausdrucks ist immer wahr: ('\ n'! = C). params.c 136.

Lassen Sie uns der Klarheit halber drei Optionen für die Entwicklung von Ereignissen analysieren:

• Das Ende der Datei ist erreicht. Das Dateiende (EOF) ist kein Leerzeichen oder Tabulator. Der Unterausdruck ('\ n'! = C) wird aufgrund der Kurzschlussauswertung nicht berechnet. Der Zyklus stoppt.
• Jedes Zeichen, das kein Leerzeichen oder Tabulator ist, wird gelesen. Der Unterausdruck ('\ n'! = C) wird aufgrund der Kurzschlussauswertung nicht berechnet. Der Zyklus stoppt.
• Lesen Sie ein Leerzeichen oder eine horizontale Registerkarte. Der Unterausdruck ('\ n'! = C) wird berechnet, aber sein Ergebnis ist immer wahr.

Mit anderen Worten, der überprüfte Code entspricht dem folgenden:

 for (c = getc (InFile); c==' ' || c == '\t'; c = getc (InFile)) 

Wir haben festgestellt, dass der Code nicht wie beabsichtigt funktioniert. Mal sehen, welche Konsequenzen dies hat.

Der Programmierer, der den isspace- Aufruf im Hauptteil der EatWhitespace- Funktion geschrieben hat, hat erwartet, dass eine Standardfunktion aufgerufen wird. Aus diesem Grund fügte er die Bedingung hinzu, dass der Zeilenvorschub LF ('\ n') nicht als Leerzeichen betrachtet werden sollte.

Daher plante der Programmierer, dass zusätzlich zu den Leerzeichen und den horizontalen Registerkarten Zeichen wie Seitenwechsel und vertikale Registerkarten übersprungen werden.

Es ist bemerkenswert, dass auch geplant war, das CR-Wagenrücklaufzeichen (0x0d, '\ r') zu überspringen. Dies geschieht nicht und der Zyklus stoppt, wenn er auf dieses Symbol trifft. Dies führt zu unangenehmen Überraschungen, wenn das Zeilentrennzeichen in der Datei die CR + LF-Sequenz ist, die auf einigen Nicht-UNIX-Systemen wie Microsoft Windows verwendet wird.

Für diejenigen, die mehr über die historischen Gründe für die Verwendung von LF oder CR + LF als Zeilentrenner erfahren möchten, ist hier der Wikipedia-Artikel " Zeilenvorschub ".

Die EatWhitespace- Funktion soll Dateien auf die gleiche Weise verarbeiten, wobei sowohl LF als auch CR + LF als Trennzeichen verwendet werden. Für den Fall von CR + LF ist dies nicht der Fall. Mit anderen Worten, wenn Ihre Datei aus der Windows-Welt stammt, haben Sie kein Glück :).

Vielleicht ist dies kein schwerwiegender Fehler, zumal GNU Midnight Commander unter UNIX-ähnlichen Betriebssystemen üblich ist, bei denen das LF-Zeichen (0x0a, '\ n') zum Übersetzen einer Zeile verwendet wird. Aufgrund solcher Kleinigkeiten treten jedoch verschiedene ärgerliche Probleme der Inkompatibilität von Daten auf, die in Linux- und Windows-Systemen vorbereitet wurden.

Der beschriebene Fehler ist insofern interessant, als er mit einer klassischen Codeüberprüfung kaum zu erkennen ist. Nicht alle Projektentwickler können die Feinheiten des Makros kennen, und es ist sehr einfach, sie zu vergessen. Dies ist ein gutes Beispiel, bei dem die statische Code-Analyse Codeüberprüfungen und andere Techniken zur Fehlersuche ergänzt.

Das Überschreiben von Standardfunktionen ist eine schlechte Praxis. Übrigens wurde kürzlich im Artikel „ Love Static Code Analysis “ ein ähnlicher Fall mit dem Makro #define sprintf std :: printf betrachtet .

Eine bessere Lösung wäre, dem Makro einen eindeutigen Namen zu geben, z. B. is_space_or_tab . Dann wäre Verwirrung unmöglich.

Möglicherweise war der Grund für die Erstellung des Makros der langsame Betrieb der Standard- Isspace- Funktion , und der Programmierer erstellte eine schnellere Version, die ausreicht, um alle erforderlichen Aufgaben zu lösen. Trotzdem ist diese Entscheidung falsch. Es wäre zuverlässiger , isspace so zu definieren, dass Sie nicht kompilierten Code erhalten. Und um die erforderliche Funktionalität in einem Makro mit einem eindeutigen Namen zu implementieren.

Vielen Dank für Ihre Aufmerksamkeit. Wir laden Sie ein, den PVS-Studio-Analysator herunterzuladen und zu testen, um Ihre Projekte zu testen. Außerdem möchten wir Sie daran erinnern, dass der Analysator kürzlich Unterstützung für die Java-Sprache hinzugefügt hat.

Wenn Sie diesen Artikel einem englischsprachigen Publikum zugänglich machen möchten, verwenden Sie bitte den Link zur Übersetzung: Andrey Karpov. Willst du einen Detektiv spielen? Finden Sie den Fehler in einer Funktion von Midnight Commander .