Bagi mereka yang ingin bermain detektif: temukan kesalahan dalam fungsi dari Midnight Commander

Kami mengundang Anda untuk mencoba menemukan kesalahan dalam fungsi yang sangat sederhana dari proyek GNU Midnight Commander. Mengapa Seperti itu saja. Itu lucu dan menarik. Meskipun tidak, kami berbohong. Sekali lagi, kami ingin menunjukkan kesalahan yang ditemukan seseorang dengan kesulitan dalam proses peninjauan kode, tetapi dengan mudah menemukan penganalisa kode statis PVS-Studio.

Baru-baru ini kami dikirimi surat yang menanyakan mengapa penganalisa menghasilkan peringatan pada fungsi EatWhitespace , kode yang diberikan di bawah ini. Sebenarnya pertanyaannya tidak begitu sederhana. Coba cari tahu sendiri apa yang salah dengan kode ini.

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 */ 

Seperti yang Anda lihat, fungsi EatWhitespace sangat kecil. Bahkan komentar pada suatu fungsi membutuhkan lebih banyak ruang daripada isi dari fungsi itu sendiri :). Sekarang beberapa detail.

Deskripsi fungsi Getc :

 int getc ( FILE * stream ); 

Fungsi mengembalikan karakter yang ditunjuk oleh indikator internal dari posisi file dari aliran yang ditentukan. Kemudian indikator menuju ke karakter berikutnya. Jika akhir file tercapai pada saat panggilan ke streaming, fungsi mengembalikan EOF dan menetapkan akhir indikator file untuk aliran ini. Jika kesalahan baca terjadi, fungsi mengembalikan nilai EOF dan menetapkan indikator kesalahan untuk aliran yang diberikan (ferror).

Deskripsi fungsi isspace :

 int isspace( int ch ); 

Fungsi memeriksa apakah karakter tersebut spasi berdasarkan klasifikasi lokal saat ini. Di lokal standar, karakter berikut adalah spasi putih:

• spasi (0x20, ``);
• perubahan halaman (0x0c, '\ f');
• LF umpan baris (0x0a, '\ n');
• carriage return CR (0x0d, '\ r');
• tab horizontal (0x09, '\ t');
• tab vertikal (0x0b, '\ v').

Nilai pengembalian Nilai bukan nol, jika karakter adalah spasi, nol jika tidak.

Fungsi EatWhitespace harus melewati semua karakter yang dianggap sebagai spasi putih, kecuali untuk umpan baris '\ n'. Alasan lain untuk berhenti membaca dari file mungkin mencapai akhir file (EOF).

Dan sekarang, mengetahui semua ini, cobalah untuk menemukan kesalahan!

Agar pembaca tidak sengaja tidak langsung melihat jawabannya, tambahkan beberapa unicorn yang menunggu.

Gambar 1. Waktu untuk mencari kesalahan. Unicorn akan menunggu.

Masih tidak melihat kesalahan?

Masalahnya adalah kita menipu pembaca tentang isspace . Haha Ini bukan fitur standar sama sekali, tetapi makro buatan sendiri. Ya, kami tidak bersalah dan membuat Anda bingung.

Gambar 2. Unicorn memberi pembaca kesan yang salah tentang apa itu ruang .

Sebenarnya, tentu saja, kita dan unicorn kita tidak bisa disalahkan. Para penulis proyek GNU Midnight Commander berkontribusi pada kebingungan dengan memutuskan untuk membuat implementasi isspace mereka sendiri di file charset.h :

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

Dengan membuat makro seperti itu, beberapa pengembang membingungkan pengembang lain. Kode ditulis dengan asumsi bahwa isspace adalah fungsi standar yang menganggap carriage return (0x0d, '\ r') sebagai salah satu karakter spasi putih.

Makro yang diimplementasikan menganggap hanya spasi dan tab sebagai karakter spasi. Mari gantikan makro dan lihat apa yang terjadi.

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

Subekspresi ('\ n'! = C) adalah redundan (redundan) karena hasilnya akan selalu benar. Penganalisa PVS-Studio memperingatkan tentang hal ini, memberikan peringatan:

V560 Bagian dari ekspresi kondisional selalu benar: ('\ n'! = C). params.c 136.

Untuk lebih jelasnya, mari kita menganalisis 3 opsi untuk pengembangan acara:

• Akhir file tercapai. End of file (EOF) bukan spasi atau tab. Subekspresi ('\ n'! = C) tidak dihitung karena evaluasi hubung singkat . Siklus berhenti.
• Karakter apa pun yang bukan spasi atau tab dibaca. Subekspresi ('\ n'! = C) tidak dihitung karena evaluasi hubung singkat. Siklus berhenti.
• Baca karakter spasi atau tab horizontal. Subekspresi ('\ n'! = C) dihitung, tetapi hasilnya akan selalu benar.

Dengan kata lain, kode yang ditinjau setara dengan ini:

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

Kami menemukan bahwa kode tidak berfungsi sebagaimana dimaksud. Mari kita lihat sekarang apa konsekuensi ini.

Programmer yang menulis panggilan isspace di tubuh fungsi EatWhitespace berharap bahwa fungsi standar akan dipanggil. Itu sebabnya dia menambahkan kondisi bahwa umpan baris LF ('\ n') tidak boleh dianggap sebagai karakter spasi putih.

Oleh karena itu, programmer merencanakan bahwa selain tab spasi dan horisontal, karakter seperti perubahan halaman dan tab vertikal akan dilewati.

Perlu dicatat bahwa itu juga direncanakan untuk melewatkan karakter carriage return CR (0x0d, '\ r'). Ini tidak terjadi dan siklus berhenti ketika bertemu simbol ini. Ini akan menyebabkan kejutan yang tidak menyenangkan jika pemisah baris dalam file adalah urutan CR + LF yang digunakan pada beberapa sistem non-UNIX, seperti Microsoft Windows.

Bagi mereka yang ingin mempelajari lebih lanjut tentang alasan historis untuk menggunakan LF atau CR + LF sebagai pemisah baris, berikut adalah artikel Wikipedia " Umpan baris ".

Fungsi EatWhitespace seharusnya memproses file dengan cara yang sama, di mana LF dan CR + LF digunakan sebagai pemisah. Untuk kasus CR + LF, ini tidak demikian. Dengan kata lain, jika file Anda berasal dari dunia Windows, maka Anda kurang beruntung :).

Mungkin ini bukan kesalahan serius, terutama karena GNU Midnight Commander adalah umum pada sistem operasi mirip UNIX, di mana karakter LF (0x0a, '\ n') digunakan untuk menerjemahkan suatu baris. Namun, karena hal-hal sepele seperti itu, berbagai masalah menjengkelkan ketidakcocokan data yang disiapkan di Linux dan sistem Windows muncul.

Kesalahan yang dijelaskan menarik karena hampir tidak mungkin terdeteksi dengan ulasan kode klasik. Tidak semua pengembang proyek dapat mengetahui seluk-beluk makro, dan melupakannya sangat mudah. Ini adalah contoh yang baik di mana analisis kode statis melengkapi ulasan kode dan teknik pencarian kesalahan lainnya.

Mengesampingkan fungsi standar adalah praktik yang buruk. Ngomong-ngomong, baru-baru ini di artikel " Love Static Code Analysis " kasus serupa dianggap dengan makro #define sprintf std :: printf .

Solusi yang lebih baik adalah memberi makro nama unik, misalnya, is_space_or_tab . Maka kebingungan tidak mungkin terjadi.

Mungkin alasan untuk membuat makro adalah lambatnya fungsi fungsi isspace standar , dan programmer membuat versi yang lebih cepat, cukup untuk menyelesaikan semua tugas yang diperlukan. Namun tetap saja, keputusan ini salah. Akan lebih andal untuk mendefinisikan isspace sedemikian rupa sehingga Anda mendapatkan kode yang tidak dikompilasi. Dan untuk mengimplementasikan fungsionalitas yang diperlukan dalam makro dengan nama yang unik.

Terima kasih atas perhatian anda Kami mengundang Anda untuk mengunduh dan mencoba penganalisa PVS-Studio untuk menguji proyek Anda. Selain itu, kami mengingatkan Anda bahwa baru-baru ini penganalisis telah menambahkan dukungan untuk bahasa Java.

Jika Anda ingin berbagi artikel ini dengan audiens yang berbahasa Inggris, silakan gunakan tautan ke terjemahan: Andrey Karpov. Ingin Bermain Detektif? Temukan Bug dalam Fungsi dari Komandan Tengah Malam .