Clever Geek Handbook

Analisis Kode Statis Koleksi Perpustakaan PMDK Intel dan Kesalahan Yang Bukan Kesalahan

PVS-Studio, PMDK


Kami ditawari untuk memeriksa koleksi pustaka PMDK terbuka yang dirancang untuk mengembangkan dan men-debug aplikasi dengan dukungan memori non-volatile dengan penganalisis PVS-Studio. Sebenarnya kenapa tidak. Selain itu, ini adalah proyek kecil dalam C dan C ++ dengan ukuran basis kode total sekitar 170 KLOC, tidak termasuk komentar. Artinya, meninjau hasil analisis tidak akan memakan banyak waktu dan tenaga. Ayo pergi.



Alat PVS-Studio versi 7.08 akan digunakan untuk menganalisis kode sumber. Pembaca blog kita, tentu saja, sudah lama akrab dengan alat kita, dan saya tidak akan membahasnya. Bagi mereka yang datang kepada kami untuk pertama kalinya, saya sarankan untuk merujuk ke artikel " Bagaimana cara cepat melihat peringatan menarik yang diberikan penganalisis PVS-Studio untuk kode C dan C ++? " Dan coba penganalisis versi uji coba gratis.



Kali ini saya akan melihat ke dalam proyek PMDK dan memberi tahu Anda tentang kesalahan dan kekurangan yang saya perhatikan. Dalam hati saya, jumlahnya tidak banyak, yang menunjukkan kualitas yang baik dari kode proyek ini. Di antara hal-hal yang menarik, kami dapat mencatat bahwa beberapa fragmen dari kode yang salah ditemukan, namun berfungsi dengan benar :). Yang saya maksud akan menjadi lebih jelas dari narasi selanjutnya.



Singkatnya, PMDK adalah kumpulan pustaka dan alat sumber terbuka yang dirancang untuk menyederhanakan pengembangan, debugging, dan pengelolaan aplikasi yang mendukung memori non-volatil. Lebih detail di sini: Pengenalan PMDK . Sumber di sini: pmdk .



Mari kita lihat kesalahan dan kekurangan apa yang bisa saya temukan di dalamnya. Saya harus segera mengatakan bahwa saya tidak selalu memperhatikan ketika menganalisis laporan dan bisa saja melewatkan banyak hal. Oleh karena itu, saya mendorong penulis proyek untuk tidak secara eksklusif dipandu oleh artikel ini saat memperbaiki cacat, tetapi untuk memeriksa ulang kodenya sendiri. Dan untuk menulis artikel, apa yang saya tulis, melihat daftar peringatan, sudah cukup bagi saya :).



Kode yang salah



Ukuran alokasi memori



Tidak jarang programmer menghabiskan waktu untuk men-debug kode ketika program tidak berperilaku sebagaimana mestinya. Namun, terkadang ada situasi ketika program bekerja dengan benar, tetapi kode mengandung kesalahan. Pemrogram hanya beruntung, dan kesalahan tidak muncul dengan sendirinya. Dalam proyek PMDK, saya menghadapi beberapa situasi yang menarik sekaligus dan oleh karena itu memutuskan untuk menggabungkannya dalam bab terpisah. 



int main(int argc, char *argv[])
{
  ....
  struct pool *pop = malloc(sizeof(pop));
  ....
}


Peringatan PVS-Studio: V568 Aneh bahwa operator 'sizeof ()' mengevaluasi ukuran pointer ke kelas, tetapi bukan ukuran objek kelas 'pop'. util_ctl.c 717



Kesalahan ketik klasik yang menyebabkan jumlah memori yang dialokasikan salah. Operator sizeof tidak akan mengembalikan ukuran struktur, tetapi ukuran penunjuk ke struktur ini. Opsi yang benar adalah: 



struct pool *pop = malloc(sizeof(pool));


atau 



struct pool *pop = malloc(sizeof(*pop));


Namun, kode yang salah ditulis ini bekerja dengan baik. Intinya adalah bahwa struktur kumpulan berisi tepat satu penunjuk: 



struct pool {
  struct ctl *ctl;
};


Ternyata strukturnya memakan banyak penunjuk. Semuanya bagus.



Panjang garis



Mari kita lanjutkan ke kasus berikutnya, di mana kesalahan dibuat lagi menggunakan operator sizeof . 



typedef void *(*pmem2_memcpy_fn)(void *pmemdest, const void *src, size_t len,
    unsigned flags);

static const char *initial_state = "No code.";

static int
test_rwx_prot_map_priv_do_execute(const struct test_case *tc,
  int argc, char *argv[])
{
  ....
  char *addr_map = pmem2_map_get_address(map);
  map->memcpy_fn(addr_map, initial_state, sizeof(initial_state), 0);
  ....
}


Peringatan PVS-Studio: V579 [CWE-687] Fungsi memcpy_fn menerima penunjuk dan ukurannya sebagai argumen. Itu mungkin kesalahan. Periksa argumen ketiga. pmem2_map_prot.c 513 Penunjuk



ke fungsi salinan khusus digunakan untuk menyalin string. Perhatikan panggilan ke fungsi ini, atau lebih tepatnya ke argumen ketiganya.



Programmer mengasumsikan bahwa sizeof Operator akan menghitung ukuran literal string yang. Tapi, nyatanya ukuran pointer dihitung lagi.



Untungnya, string tersebut terdiri dari 8 karakter, dan ukurannya sama dengan ukuran pointer jika aplikasi 64-bit sedang dibangun. Akibatnya, semua 8 karakter dari string "Tidak ada kode". akan berhasil disalin.



Faktanya, situasinya bahkan lebih rumit dan menarik. Interpretasi kesalahan ini tergantung pada apakah Anda ingin menyalin terminal nol atau tidak. Mari pertimbangkan dua skenario.



Skenario 1. Itu perlu untuk menyalin terminal nol. Maka saya salah, dan ini sama sekali bukan kesalahan tidak berbahaya yang tidak terwujud dengan sendirinya. Bukan 9 byte yang disalin, tetapi hanya 8. Tidak ada terminal nol, dan konsekuensinya tidak dapat diprediksi. Dalam hal ini, kode dapat dikoreksi dengan mengubah definisi string konstanta initial_state sebagai berikut: 



static const char initial_state [] = "No code.";


Sekarang nilai sizeof (initial_state) adalah 9.



Skenario 2. Terminal nol tidak diperlukan sama sekali. Misalnya, di bawah ini Anda dapat melihat baris kode berikut: 



UT_ASSERTeq(memcmp(addr_map, initial_state, strlen(initial_state)), 0);


Seperti yang Anda lihat, fungsi strlen akan mengembalikan nilai 8 dan terminal nol tidak termasuk dalam perbandingan. Maka itu benar-benar keberuntungan dan semuanya baik-baik saja.



Sedikit bergeser



Contoh selanjutnya terkait dengan operasi pergeseran bitwise. 



static int
clo_parse_single_uint(struct benchmark_clo *clo, const char *arg, void *ptr)
{
  ....
  uint64_t tmax = ~0 >> (64 - 8 * clo->type_uint.size);
  ....
}


Peringatan PVS-Studio: V610 [CWE-758] Perilaku tidak ditentukan. Periksa operator shift '>>'. Operan kiri '~ 0' negatif. clo.cpp 205



Hasil dari menggeser nilai negatif ke kanan tergantung pada implementasi compiler. Oleh karena itu, meskipun kode tersebut dapat bekerja dengan benar dan seperti yang diharapkan dalam semua mode kompilasi aplikasi yang ada saat ini, itu masih keberuntungan.



Prioritas operasi



Dan pertimbangkan kasus terakhir yang terkait dengan prioritas operasi. 



#define BTT_CREATE_DEF_SIZE  (20 * 1UL << 20) /* 20 MB */


Peringatan PVS-Studio: V634 [CWE-783] Prioritas operasi '*' lebih tinggi daripada operasi '<<'. Ada kemungkinan bahwa tanda kurung harus digunakan dalam ekspresi. bttcreate.c 204



Untuk mendapatkan konstanta sebesar 20 MB, programmer memutuskan untuk melakukan hal berikut:



  • Bergeser 1 kali 20 bit untuk mendapatkan nilai 1048576, mis. 1 MB.
  • Dikalikan 1 MB dengan 20.


Dengan kata lain, programmer berpikir bahwa perhitungan dilakukan seperti ini: (20 * (1UL << 20))



Namun kenyataannya, prioritas operator perkalian lebih tinggi daripada operator shift, dan ekspresi dievaluasi seperti ini: ((20 * 1UL) << 20).



Setuju, programmer hampir tidak ingin ekspresi dievaluasi dalam urutan seperti itu. Tidak ada gunanya mengalikan 20 dengan 1. Jadi ini adalah kasus ketika kode tidak berfungsi seperti yang diinginkan programmer.



Tapi kesalahan ini tidak akan muncul dengan sendirinya. Tidak peduli bagaimana Anda menulis:



  • (20 * 1UL << 20)
  • (20 * (1UL << 20))
  • ((20 * 1UL) << 20)


Hasilnya selalu sama ! Nilai yang diinginkan selalu 20971520 dan program bekerja dengan baik.



Kesalahan lainnya



Tanda kurung di tempat yang salah



#define STATUS_INFO_LENGTH_MISMATCH 0xc0000004

static void
enum_handles(int op)
{
  ....
  NTSTATUS status;
  while ((status = NtQuerySystemInformation(
      SystemExtendedHandleInformation,
      hndl_info, hi_size, &req_size)
        == STATUS_INFO_LENGTH_MISMATCH)) {
    hi_size = req_size + 4096;
    hndl_info = (PSYSTEM_HANDLE_INFORMATION_EX)REALLOC(hndl_info,
        hi_size);
  }
  UT_ASSERT(status >= 0);
  ....
}


Peringatan PVS-Studio: V593 [CWE-783] Pertimbangkan untuk meninjau ekspresi dari jenis 'A = B == C'. Ekspresi dihitung sebagai berikut: 'A = (B == C)'. ut.c 641



Perhatikan baik-baik di sini: 



while ((status = NtQuerySystemInformation(....) == STATUS_INFO_LENGTH_MISMATCH))


Pemrogram ingin menyimpan dalam variabel status nilai yang dikembalikan oleh fungsi NtQuerySystemInformation , dan kemudian membandingkannya dengan konstanta.



Programmer kemungkinan besar mengetahui bahwa operator pembanding (==) memiliki prioritas yang lebih tinggi daripada operator penugasan (=), dan oleh karena itu tanda kurung harus digunakan. Tapi dia menyegel dirinya sendiri dan meletakkannya di tempat yang salah. Akibatnya, tanda kurung tidak membantu sama sekali. Kode yang benar: 



while ((status = NtQuerySystemInformation(....)) == STATUS_INFO_LENGTH_MISMATCH)


Karena kesalahan ini, makro UT_ASSERT tidak akan pernah berfungsi. Memang variabel status selalu berisi hasil perbandingan, yaitu false (0) atau true (1). Oleh karena itu kondisi ([0..1]> = 0) selalu benar.



Potensi kebocoran memori



static enum pocli_ret
pocli_args_obj_root(struct pocli_ctx *ctx, char *in, PMEMoid **oidp)
{
  char *input = strdup(in);
  if (!input)
    return POCLI_ERR_MALLOC;

  if (!oidp)
    return POCLI_ERR_PARS;
  ....
}


Peringatan PVS-Studio: V773 [CWE-401] Fungsi ini keluar tanpa melepaskan penunjuk 'input'. Kebocoran memori mungkin terjadi. pmemobjcli.c 238



Jika oidp ternyata pointer nol, salinan string yang dibuat dengan memanggil fungsi strdup akan hilang . Taruhan terbaik Anda adalah menunda pemeriksaan sebelum mengalokasikan memori: 



static enum pocli_ret
pocli_args_obj_root(struct pocli_ctx *ctx, char *in, PMEMoid **oidp)
{
  if (!oidp)
    return POCLI_ERR_PARS;

  char *input = strdup(in);
  if (!input)
    return POCLI_ERR_MALLOC;
  ....
}


Atau, Anda dapat mengosongkan memori secara eksplisit: 



static enum pocli_ret
pocli_args_obj_root(struct pocli_ctx *ctx, char *in, PMEMoid **oidp)
{
  char *input = strdup(in);
  if (!input)
    return POCLI_ERR_MALLOC;

  if (!oidp)
  {
    free(input);
    return POCLI_ERR_PARS;
  }
  ....
}


Potensi melimpah



typedef long long os_off_t;

void
do_memcpy(...., int dest_off, ....., size_t mapped_len, .....)
{
  ....
  LSEEK(fd, (os_off_t)(dest_off + (int)(mapped_len / 2)), SEEK_SET);
  ....
}


Peringatan PVS-Studio: V1028 [CWE-190] Kemungkinan meluap. Pertimbangkan operan casting, bukan hasilnya. memcpy_common.c 62 Tidaklah masuk akal untuk



secara eksplisit menampilkan hasil penambahan ke tipe os_off_t . Pertama, ini tidak melindungi dari potensi luapan yang dapat terjadi saat menambahkan dua nilai int . Kedua, hasil penambahan akan secara implisit diperluas ke tipe os_off_t . Pengecoran tipe eksplisit hanya berlebihan.



Saya pikir akan lebih tepat menulis seperti ini: 



LSEEK(fd, dest_off + (os_off_t)(mapped_len) / 2, SEEK_SET);


Di sini nilai unsigned dari tipe size_t diubah menjadi nilai yang ditandatangani (sehingga tidak ada peringatan dari compiler). Dan pada saat yang sama, pasti tidak akan ada luapan selama penambahan.



Perlindungan overflow salah



static DWORD
get_rel_wait(const struct timespec *abstime)
{
  struct __timeb64 t;
  _ftime64_s(&t);
  time_t now_ms = t.time * 1000 + t.millitm;
  time_t ms = (time_t)(abstime->tv_sec * 1000 +
    abstime->tv_nsec / 1000000);

  DWORD rel_wait = (DWORD)(ms - now_ms);

  return rel_wait < 0 ? 0 : rel_wait;
}


Peringatan PVS-Studio: V547 [CWE-570] Ekspresi 'rel_wait <0' selalu salah. Jenis nilai unsigned tidak pernah <0. os_thread_windows.c 359



Saya tidak begitu jelas tentang situasi mana yang harus dilindungi oleh pemeriksaan, tetapi tetap tidak berfungsi. Variabel rel_wait adalah tipe DWORD yang tidak bertanda tangan . Artinya perbandingan rel_wait <0 tidak ada artinya, karena hasilnya selalu benar.



Kurangnya verifikasi bahwa memori telah berhasil dialokasikan



Memeriksa bahwa memori dialokasikan dilakukan dengan menggunakan menegaskan macro , yang melakukan apa-apa jika versi Release dari aplikasi dikompilasi. Jadi kita dapat mengatakan bahwa tidak ada penanganan situasi di mana fungsi malloc mengembalikan NULL . Contoh: 



static void
remove_extra_node(TOID(struct tree_map_node) *node)
{
  ....
  unsigned char *new_key = (unsigned char *)malloc(new_key_size);
  assert(new_key != NULL);
  memcpy(new_key, D_RO(tmp)->key, D_RO(tmp)->key_size);
  ....
}


Peringatan PVS-Studio: V575 [CWE-628] Pointer null potensial diteruskan ke fungsi 'memcpy'. Periksa argumen pertama. Periksa baris: 340, 338. rtree_map.c 340



Di tempat lain bahkan tidak ada assert : 



static void
calc_pi_mt(void)
{
  ....
  HANDLE *workers = (HANDLE *) malloc(sizeof(HANDLE) * pending);
  for (i = 0; i < pending; ++i) {
    workers[i] = CreateThread(NULL, 0, calc_pi,
      &tasks[i], 0, NULL);
    if (workers[i] == NULL)
      break;
  }
  ....
}


Peringatan PVS-Studio: V522 [CWE-690] Mungkin ada dereferensi dari 'pekerja' penunjuk null potensial. Periksa baris: 126, 124. pi.c 126



Saya menghitung setidaknya 37 potongan kode tersebut. Jadi saya tidak melihat alasan untuk mencantumkan semuanya di artikel.



Sekilas, kurangnya pemeriksaan bisa dianggap hanya kecerobohan dan mengatakan bahwa ini adalah kode dengan bau. Saya tidak setuju dengan posisi ini. Pemrogram meremehkan bahaya tidak adanya pemeriksaan semacam itu. Pointer null tidak selalu langsung memanifestasikan dirinya sebagai program crash ketika mencoba untuk membedakannya. Konsekuensinya bisa lebih aneh dan berbahaya, terutama dalam program multi-thread. Untuk memahami lebih detail tentang masalah ini dan mengapa pemeriksaan diperlukan, saya sangat menyarankan semua orang untuk membaca artikel "Mengapa penting untuk memeriksa apa yang kembali malloc ".



Kode berbau



Double Call CloseHandle



static void
prepare_map(struct pmem2_map **map_ptr,
  struct pmem2_config *cfg, struct pmem2_source *src)
{
  ....
  HANDLE mh = CreateFileMapping(....);
  ....
  UT_ASSERTne(CloseHandle(mh), 0);
  ....
}


Peringatan PVS-Studio: V586 [CWE-675] Fungsi 'CloseHandle' dipanggil dua kali untuk membatalkan alokasi sumber daya yang sama. pmem2_map.c 76



Melihat kode ini dan peringatan PVS-Studio, jelas bahwa tidak ada yang jelas. Di mana saya dapat memanggil CloseHandle lagi ? Untuk menemukan jawabannya, mari kita lihat implementasi makro UT_ASSERTne . 



#define UT_ASSERTne(lhs, rhs)\
  do {\
    /* See comment in UT_ASSERT. */\
    if (__builtin_constant_p(lhs) && __builtin_constant_p(rhs))\
      UT_ASSERT_COMPILE_ERROR_ON((lhs) != (rhs));\
    UT_ASSERTne_rt(lhs, rhs);\
  } while (0)


Itu tidak menjadi lebih jelas. Apakah UT_ASSERT_COMPILE_ERROR_ON itu ? Apa UT_ASSERTne_rt itu ?



Saya tidak akan mengacaukan artikel dengan deskripsi setiap makro dan menyiksa pembaca, memaksanya untuk memasukkan satu makro ke makro lain di kepalanya. Mari kita lihat sekaligus versi terakhir dari kode yang dibuka yang diambil dari file yang diproses sebelumnya. 



do {
  if (0 && 0) (void)((CloseHandle(mh)) != (0));
  ((void)(((CloseHandle(mh)) != (0)) ||
    (ut_fatal(".....", 76, __FUNCTION__, "......: %s (0x%llx) != %s (0x%llx)",
              "CloseHandle(mh)", (unsigned long long)(CloseHandle(mh)), "0",
              (unsigned long long)(0)), 0))); } while (0);


Mari kita hapus kondisi selalu salah (0 && 0) dan, secara umum, semua yang tidak relevan. Ternyata: 



((void)(((CloseHandle(mh)) != (0)) ||
  (ut_fatal(...., "assertion failure: %s (0x%llx) != %s (0x%llx)",
            ....., (unsigned long long)(CloseHandle(mh)), .... ), 0)));


Pegangannya tertutup. Jika kesalahan terjadi, pesan debug dibuat dan, untuk mendapatkan kode kesalahan lagi, CloseHandle dipanggil untuk pegangan tidak valid yang sama.



Kesalahan, semacam, dan tidak. Karena pegangannya tidak valid, maka tidak masalah jika fungsi CloseHandle dipanggil dua kali untuk itu . Namun, kode ini tidak berbau. Akan lebih benar secara ideologis untuk memanggil fungsi hanya sekali dan menyimpan status yang dikembalikannya, kemudian, jika perlu, tampilkan nilainya dalam pesan.



Ketidakkonsistenan antarmuka implementasi (penghapusan const)



static int
status_push(PMEMpoolcheck *ppc, struct check_status *st, uint32_t question)
{
  ....
  } else {
    status_msg_info_and_question(st->msg);            // <=
    st->question = question;
    ppc->result = CHECK_RESULT_ASK_QUESTIONS;
    st->answer = PMEMPOOL_CHECK_ANSWER_EMPTY;
    PMDK_TAILQ_INSERT_TAIL(&ppc->data->questions, st, next);
  }
  ....
}


Penganalisis menampilkan pesan: V530 [CWE-252] Nilai kembali dari fungsi 'status_msg_info_and_question' diperlukan untuk digunakan. check_util.c 293



Alasannya adalah bahwa fungsi status_msg_info_and_question , dari sudut pandang penganalisis, tidak mengubah status objek di luarnya, termasuk string konstan yang diteruskan. Itu. fungsi hanya menghitung sesuatu dan mengembalikan hasilnya. Dan jika demikian, maka aneh untuk tidak menggunakan hasil yang dikembalikan fungsi ini. Dan, meskipun penganalisisnya salah kali ini, ia menunjuk ke kode dengan bau. Mari kita lihat bagaimana fungsi yang disebut status_msg_info_and_question bekerja . 



static inline int
status_msg_info_and_question(const char *msg)
{
  char *sep = strchr(msg, MSG_SEPARATOR);
  if (sep) {
    *sep = ' ';
    return 0;
  }
  return -1;
}


Saat memanggil fungsi strchr , penghapusan implisit dari konstanta terjadi. Faktanya adalah bahwa di C dinyatakan seperti ini: 



char * strchr ( const char *, int );


Bukan solusi terbaik. Tapi bahasa C adalah :).



Penganalisis menjadi bingung dan tidak memahami bahwa string yang diberikan sebenarnya berubah. Jika demikian, maka nilai pengembalian bukanlah hal yang terpenting dan Anda tidak dapat menggunakannya.



Namun, meskipun penganalisis bingung, penganalisis menunjuk ke kode dengan bau. Apa yang membingungkan parser juga dapat membingungkan orang yang menyimpan kode. Akan lebih baik untuk mendeklarasikan fungsi tersebut secara lebih jujur ​​dengan menghapus const : 



static inline int
status_msg_info_and_question(char *msg)
{
  char *sep = strchr(msg, MSG_SEPARATOR);
  if (sep) {
    *sep = ' ';
    return 0;
  }
  return -1;
}


Jadi niat segera menjadi lebih jelas, dan penganalisis akan diam.



Kode yang terlalu rumit



static struct memory_block
heap_coalesce(struct palloc_heap *heap,
  const struct memory_block *blocks[], int n)
{
  struct memory_block ret = MEMORY_BLOCK_NONE;

  const struct memory_block *b = NULL;
  ret.size_idx = 0;
  for (int i = 0; i < n; ++i) {
    if (blocks[i] == NULL)
      continue;
    b = b ? b : blocks[i];
    ret.size_idx += blocks[i] ? blocks[i]->size_idx : 0;
  }
  ....
}


Peringatan PVS-Studio: V547 [CWE-571] Ekspresi 'blok [i]' selalu benar. heap.c 1054



Jika blok [i] == NULL , maka pernyataan lanjutkan akan dipicu dan loop akan memulai iterasi berikutnya. Oleh karena itu, memeriksa ulang blok elemen [i] tidak masuk akal dan operator terner tidak berguna. Kode dapat disederhanakan: 



....
for (int i = 0; i < n; ++i) {
  if (blocks[i] == NULL)
    continue;
  b = b ? b : blocks[i];
  ret.size_idx += blocks[i]->size_idx;
}
....


Penggunaan yang mencurigakan dari pointer nol



void win_mmap_fini(void)
{
  ....
  if (mt->BaseAddress != NULL)
    UnmapViewOfFile(mt->BaseAddress);
  size_t release_size =
    (char *)mt->EndAddress - (char *)mt->BaseAddress;
  void *release_addr = (char *)mt->BaseAddress + mt->FileLen;
  mmap_unreserve(release_addr, release_size - mt->FileLen);
  ....
}


Peringatan PVS-Studio: V1004 [CWE-119] Pointer '(char *) mt-> BaseAddress' digunakan dengan tidak aman setelah diverifikasi terhadap nullptr. Baris centang: 226, 235. win_mmap.c 235



Pointer mt-> BaseAddress bisa kosong, yang dibuktikan dengan cek: 



if (mt->BaseAddress != NULL)


Namun, di bawah penunjuk ini sudah digunakan dalam operasi aritmatika tanpa pemeriksaan. Contohnya disini: 



size_t release_size =
  (char *)mt->EndAddress - (char *)mt->BaseAddress;


Beberapa nilai integer besar akan diterima, yang sebenarnya adalah nilai dari pointer mt-> EndAddress . Ini mungkin bukan bug, tetapi semuanya terlihat sangat mencurigakan, dan menurut saya kode tersebut harus diperiksa ulang. Baunya terletak pada fakta bahwa kode tersebut tidak dapat dipahami dan jelas tidak memiliki komentar penjelasan.



Nama pendek variabel global



Saya percaya bahwa kode ini berbau jika berisi variabel global dengan nama pendek. Sangat mudah untuk menutup dan secara tidak sengaja tidak menggunakan variabel lokal, tetapi variabel global di beberapa fungsi. Contoh: 



static struct critnib *c;


Peringatan PVS-Studio untuk variabel seperti itu:



  • V707 Memberi nama pendek ke variabel global dianggap praktik yang buruk. Disarankan untuk mengganti nama variabel 'ri'. map.c 131
  • V707 Memberi nama pendek ke variabel global dianggap praktik yang buruk. Disarankan untuk mengganti nama variabel 'c'. obj_critnib_mt.c 56
  • V707 Memberi nama pendek ke variabel global dianggap praktik yang buruk. Disarankan untuk mengganti nama variabel 'Id'. obj_list.h 68
  • V707 Memberi nama pendek ke variabel global dianggap praktik yang buruk. Disarankan untuk mengganti nama variabel 'Id'. obj_list.c 34


Aneh



PVS-Studio: Kode aneh dalam suatu fungsi


Kode paling aneh yang saya temukan ada di fungsi do_memmove . Penganalisis memberikan dua hal positif, yang menunjukkan kesalahan yang sangat serius, atau saya tidak mengerti apa yang saya maksud. Karena kodenya sangat aneh, saya memutuskan untuk mempertimbangkan peringatan yang dikeluarkan di bagian artikel yang terpisah. Jadi peringatan pertama dikeluarkan di sini. 



void
do_memmove(char *dst, char *src, const char *file_name,
    size_t dest_off, size_t src_off, size_t bytes,
    memmove_fn fn, unsigned flags, persist_fn persist)
{
  ....
  /* do the same using regular memmove and verify that buffers match */
  memmove(dstshadow + dest_off, dstshadow + dest_off, bytes / 2);
  verify_contents(file_name, 0, dstshadow, dst, bytes);
  verify_contents(file_name, 1, srcshadow, src, bytes);
  ....
}


Peringatan PVS-Studio: V549 [CWE-688] Argumen pertama fungsi 'memmove' sama dengan argumen kedua. memmove_common.c 71



Perhatikan bahwa argumen pertama dan kedua dari fungsi tersebut adalah sama. Jadi, sebenarnya fungsinya tidak melakukan apa-apa. Pilihan apa yang muncul di benak saya:



  • Saya ingin "menyentuh" ​​blok memori. Tetapi apakah ini akan terjadi dalam kenyataan? Akankah kompiler pengoptimalan menghapus kode yang menyalin blok memori ke dalamnya?
  • Ini adalah semacam pengujian unit untuk fungsi memmove .
  • Kode tersebut mengandung kesalahan ketik.


Dan berikut ini cuplikan yang sama anehnya dalam fungsi yang sama: 



void
do_memmove(char *dst, char *src, const char *file_name,
    size_t dest_off, size_t src_off, size_t bytes,
    memmove_fn fn, unsigned flags, persist_fn persist)
{
  ....
  /* do the same using regular memmove and verify that buffers match */
  memmove(dstshadow + dest_off, srcshadow + src_off, 0);
  verify_contents(file_name, 2, dstshadow, dst, bytes);
  verify_contents(file_name, 3, srcshadow, src, bytes);
  ....
}


Peringatan PVS-Studio: V575 [CWE-628] Fungsi 'memmove' memproses elemen '0'. Periksa argumen ketiga. memmove_common.c 82



Fungsi bergerak 0 byte. Apa itu? Tes unit? Salah ketik?



Bagi saya, kode ini tidak bisa dimengerti dan aneh.



Mengapa menggunakan penganalisis kode?



Tampaknya karena hanya sedikit kesalahan yang ditemukan, pengenalan penganalisis ke dalam proses pengembangan kode tidak masuk akal. Namun tujuan penggunaan alat analisis statis bukanlah pada pemeriksaan satu kali, tetapi dalam pendeteksian kesalahan secara teratur pada tahap penulisan kode. Jika tidak, kesalahan ini dideteksi dengan cara yang lebih mahal dan lebih lambat (debugging, pengujian, ulasan pengguna, dan sebagainya). Ide ini dijelaskan secara lebih rinci dalam artikel " Kesalahan yang tidak dapat ditemukan oleh analisis kode statis, karena tidak digunakan ", yang saya sarankan untuk dipelajari. Dan kemudian datang ke situs web kami untuk mengunduh dan mencoba PVS-Studio untuk memeriksa proyek Anda.



Terima kasih atas perhatian Anda!





Jika Anda ingin berbagi artikel ini dengan audiens berbahasa Inggris, silakan gunakan tautan terjemahan: Andrey Karpov. Analisis kode statis dari koleksi perpustakaan PMDK oleh Intel dan kesalahan yang bukan kesalahan sebenarnya .


More articles:


All Articles