Ingin Bermain Detektif? Temukan Bug dalam Fungsi dari Komandan Tengah Malam

bug

Pada artikel ini, kami mengundang Anda untuk mencoba menemukan bug dalam fungsi yang sangat sederhana dari proyek GNU Midnight Commander. Mengapa Tanpa alasan tertentu. Hanya untuk bersenang-senang. Baiklah, itu bohong. Kami sebenarnya ingin menunjukkan kepada Anda bug lain yang sulit ditemukan oleh reviewer manusia dan penganalisa kode statis PVS-Studio dapat menangkapnya tanpa usaha.

Seorang pengguna mengirimi kami email beberapa hari yang lalu, menanyakan mengapa ia mendapat peringatan tentang fungsi EatWhitespace (lihat kode di bawah). Pertanyaan ini tidak sepele seperti kelihatannya. 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, EatWhitespace adalah fungsi kecil; tubuhnya bahkan lebih kecil dari komentar di atasnya :). Sekarang, mari kita periksa beberapa detail.

Berikut deskripsi fungsi getc :

 int getc ( FILE * stream ); 

Mengembalikan karakter yang saat ini ditunjukkan oleh indikator posisi file internal dari aliran yang ditentukan. Indikator posisi file internal kemudian ditingkatkan ke karakter berikutnya. Jika aliran ada di akhir file ketika dipanggil, fungsi mengembalikan EOF dan menetapkan indikator akhir file untuk aliran. Jika kesalahan baca terjadi, fungsi mengembalikan EOF dan menetapkan indikator kesalahan untuk aliran (ferror).

Dan inilah deskripsi fungsi isspace :

 int isspace( int ch ); 

Memeriksa apakah karakter yang diberikan adalah karakter spasi yang diklasifikasikan oleh lokal C yang saat ini diinstal. Di lokal default, karakter spasi putih adalah sebagai berikut:

  • spasi (0x20, '');
  • umpan formulir (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 tersebut adalah karakter spasi putih; nol sebaliknya.

Fungsi EatWhitespace diharapkan untuk melewati semua karakter spasi kecuali feed baris '\ n'. Fungsi ini juga akan berhenti membaca dari file ketika bertemu End of file (EOF).

Sekarang setelah Anda mengetahui semua itu, cobalah untuk menemukan bug!

Dua unicorn di bawah ini akan memastikan Anda tidak secara tidak sengaja mengintip komentar tersebut.

Gambar 1. Waktu untuk pencarian bug. Unicorn sedang menunggu.


Gambar 1. Waktu untuk pencarian bug. Unicorn sedang menunggu.

Masih belum beruntung?

Nah, Anda tahu, itu karena kami telah berbohong kepada Anda tentang ruang . Bwa-ha-ha! Ini sama sekali bukan fungsi standar - ini adalah makro khusus. Ya, kami penjahat dan kami membuat Anda bingung.

Gambar 2. Unicorn membingungkan pembaca tentang isspace.


Gambar 2. Unicorn membingungkan pembaca tentang isspace .

Bukan kita atau unicorn kita yang harus disalahkan, tentu saja. Kesalahan untuk semua kebingungan terletak pada penulis proyek GNU Midnight Commander, yang membuat implementasi isspace mereka sendiri di file charset.h:

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

Dengan makro ini, penulis bingung pengembang lain. Kode ditulis dengan asumsi bahwa isspace adalah fungsi standar, yang menganggap carriage return (0x0d, '\ r') karakter spasi.

Makro kustom, pada gilirannya, memperlakukan hanya karakter spasi dan tab sebagai karakter spasi putih. Mari gantikan makro itu dan lihat apa yang terjadi.

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

Sub-ekspresi ('\ n'! = C) tidak perlu (redundan) karena itu akan selalu bernilai true. Itulah yang PVS-Studio memperingatkan Anda tentang dengan mengeluarkan peringatan:

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

Untuk memperjelasnya, mari kita periksa 3 hasil yang mungkin:

  • Akhir file tercapai. EOF bukan karakter spasi atau tab. Sub-ekspresi ('\ n'! = C) tidak dievaluasi karena evaluasi hubung singkat . Loop berakhir.
  • Fungsi telah membaca beberapa karakter yang bukan karakter spasi atau tab. Sub-ekspresi ('\ n'! = C) tidak dievaluasi karena evaluasi hubung singkat. Loop berakhir.
  • Fungsi telah membaca karakter tab spasi atau horizontal. Sub-ekspresi ('\ n'! = C) dievaluasi, tetapi hasilnya selalu benar.

Dengan kata lain, kode di atas setara dengan yang berikut:

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

Kami telah menemukan bahwa itu tidak bekerja dengan cara yang diinginkan. Sekarang mari kita lihat apa implikasinya.

Seorang pengembang, yang menulis panggilan isspace di tubuh fungsi EatWhitespace berharap bahwa fungsi standar akan dipanggil. Itu sebabnya mereka menambahkan kondisi yang mencegah karakter LF ('\ n') diperlakukan sebagai karakter spasi.

Ini berarti, selain karakter tab spasi dan horisontal, mereka juga berencana untuk melewatkan umpan form dan karakter tab vertikal.

Apa yang lebih luar biasa adalah bahwa mereka ingin karakter carriage return (0x0d, '\ r') dilewati juga. Itu tidak terjadi meskipun - loop berakhir ketika bertemu dengan karakter ini. Program akan berakhir secara tidak terduga jika baris baru diwakili oleh urutan CR + LF, yang merupakan tipe yang digunakan dalam beberapa sistem non-UNIX seperti Microsoft Windows.

Untuk detail lebih lanjut tentang alasan historis untuk menggunakan LF atau CR + LF sebagai karakter baris baru, lihat halaman Wikipedia " Baris Baru ".

Fungsi EatWhitespace dimaksudkan untuk memproses file dengan cara yang sama, apakah mereka menggunakan LF atau CR + LF sebagai karakter baris baru. Tetapi gagal dalam kasus CR + LF. Dengan kata lain, jika file Anda dari dunia Windows, Anda dalam kesulitan :).

Walaupun ini mungkin bukan bug yang serius, terutama mengingat bahwa GNU Midnight Commander digunakan dalam sistem operasi mirip UNIX, di mana LF (0x0a, '\ n') digunakan sebagai karakter baris baru, hal-hal sepele seperti itu cenderung cenderung menjengkelkan. masalah dengan kompatibilitas data yang disiapkan di Linux dan Windows.

Apa yang membuat bug ini menarik adalah bahwa Anda hampir pasti akan mengabaikannya saat melakukan tinjauan kode standar. Spesifikasi implementasi makro mudah untuk dilupakan, dan beberapa penulis proyek mungkin tidak mengetahuinya sama sekali. Ini adalah contoh yang sangat jelas tentang bagaimana analisis kode statis berkontribusi pada tinjauan kode dan teknik deteksi bug lainnya.

Mengesampingkan fungsi standar adalah praktik yang buruk. Ngomong-ngomong, kami membahas kasus serupa #define sprintf std :: printf macro di artikel terbaru " Appreciate Static Code Analysis ".

Solusi yang lebih baik adalah memberi makro nama unik, misalnya, is_space_or_tab . Ini akan membantu menghindari semua kebingungan.

Mungkin fungsi ruang isspace standar terlalu lambat dan programmer membuat versi yang lebih cepat, cukup untuk kebutuhan mereka. Tapi mereka seharusnya tidak melakukannya dengan cara itu. Solusi yang lebih aman adalah dengan mendefinisikan isspace sehingga Anda akan mendapatkan kode yang tidak dapat dikompilasi, sementara fungsi yang diinginkan dapat diimplementasikan sebagai makro dengan nama yang unik.

Terima kasih sudah membaca. Jangan ragu untuk mengunduh PVS-Studio dan coba dengan proyek Anda. Sebagai pengingat, kami sekarang mendukung Java juga.

Source: https://habr.com/ru/post/id439372/


All Articles