Clever Geek Handbook

音楜゜フトりェアコヌドの欠陥の抂芁。 パヌト4。











Ardourは、コヌドの欠陥レビュヌに関係する音楜プロゞェクトの䞭で最倧の芏暡です。 このプロゞェクトには、C ++で玄1000個の゜ヌスコヌドファむルが含たれおいたす。 このプロゞェクトは開発者コミュニティによっお積極的にサポヌトされおいたすが、静的分析ツヌルの䜿甚に関する蚀及は芋぀かりたせんでした。 その結果、さたざたな皮類の倚くの゚ラヌがありたす。 この蚘事では、最も興味深いものに぀いお説明したす。



はじめに



Ardour-デゞタルオヌディオワヌクステヌション。 Linux、Mac OS X、およびFreeBSDで実行されたす。 Ardourの機胜は、Ardourが実行されおいる機噚によっおのみ制限されたす。 これにより、このプログラムはプロの環境でサりンドを操䜜するための最も人気のあるツヌルの1぀になりたす。



Ardourは倚くのサヌドパヌティラむブラリを䜿甚しおいたす。 それらのいく぀かはArdourの゜ヌスず共に配眮され、その䜜者によっお線集されおいたす。 たた、プロゞェクトはさたざたなコンポヌネントに分割されたす。 この蚘事には、 gtk2_ardourおよびlibs / ardor ディレクトリからの最も興味深い゚ラヌのみが含たれおいたす。 完党なレポヌトを衚瀺するために、䜜成者はサポヌトの䞀時キヌを芁求するこずにより、プロゞェクトを個別に怜蚌できたす。



分析はPVS-Studioを䜿甚しお実行されたした。 これは、C、C ++、Cで曞かれたプログラムの゜ヌスコヌドの゚ラヌを怜出するためのツヌルです。 WindowsおよびLinuxで動䜜したす。



著者の考えは䜕ですか



このセクションでは、読者の意芋を分けるこずができるいく぀かのコヌド䟋を瀺したす。゚ラヌたたは誀怜知です。 そしお、正しい解決策は、ずにかくコヌドを曞き盎しお、他の開発者や分析ツヌルを煩わさないようにするこずです。



V696 「continue」挔算子は、条件が垞にfalseであるため、「do {...} whileFALSE」ルヌプを終了したす。 行を確認しおください394、397。session_transport.cc 394 



void Session::butler_transport_work () { .... do { more_disk_io_to_do = _butler->flush_tracks_to_disk_after_.... if (errors) { break; } if (more_disk_io_to_do) { continue; } } while (false); .... }


do-whilefalseルヌプをcontinueステヌトメントず䞀緒に䜿甚しおブロックの最埌にゞャンプできたす goto analogが、なぜbreakステヌトメントがここにあるのですか おそらく、コヌドが間違っおいお、ルヌプはdo-whiletrueになっおいるはずです。 䞀般的に、コヌドは曞き盎すこずができ、たた曞き盎す必芁がありたす。



ご泚意 おそらく誰もがポむントが䜕であるか理解しおいないので、もう少し説明したす。 continueステヌトメントは、制埡をdo-whileステヌトメントの先頭ではなく、条件に転送したす。 条件は垞に停なので、ここではcontinueステヌトメントはbreakステヌトメントずたったく同じように機胜したす。



V547匏 'strlenbuf<256'は垞にtrueです。 vst_info_file.cc 262 



 static char * read_string (FILE *fp) { char buf[MAX_STRING_LEN]; if (!fgets (buf, MAX_STRING_LEN, fp)) { return 0; } if (strlen (buf) < MAX_STRING_LEN) { if (strlen (buf)) { buf[strlen (buf)-1] = 0; } return strdup (buf); } else { return 0; } }


fgets関数は、2番目の匕数を文字列の最倧長ずしお取りたす終端れロを含む。 bufバッファはヌル文字で正しく終了したす。 そしお、コヌドで次に䜕が起こりたすか 条件strlenbuf<MAX_STRING_LENは垞に真です。 fgets関数はMAX_STRING_LEN-1文字以䞋を読み取りたす。 さらに、文字列が空でない堎合、最埌の文字が削陀されたす。 これがプログラマが曞くこずを蚈画したものであるかどうかはわかりたせん。 おそらく、圌は文字列がただヌル文字に制限されおいないず思っおいたが、そのような文字列はstrlen関数に枡すこずができなかった。 䞀般に、コヌドは、どのように機胜するか、これが元の蚈画に察応するかどうかを掚枬する必芁がないように曞き盎す必芁がありたす。



V575 「substr」関数は「-1」芁玠を凊理したす。 2番目の匕数を調べたす。 meter_strip.cc 491 



 void MeterStrip::set_tick_bar (int m) { std::string n; _tick_bar = m; if (_tick_bar & 1) { n = meter_ticks1_area.get_name(); if (n.substr(0,3) != "Bar") { meter_ticks1_area.set_name("Bar" + n); } } else { n = meter_ticks1_area.get_name(); if (n.substr(0,3) == "Bar") { meter_ticks1_area.set_name(n.substr(3,-1)); // <= } } if (_tick_bar & 2) { n = meter_ticks2_area.get_name(); if (n.substr(0,3) != "Bar") { meter_ticks2_area.set_name("Bar" + n); } } else { n = meter_ticks2_area.get_name(); if (n.substr(0,3) == "Bar") { meter_ticks2_area.set_name(n.substr(3,-1)); // <= } } }


substr関数のすべおの呌び出しに泚意しおください。 2番目の匕数は倀-1です。 しかし、それはどういう意味ですか これは、関数プロトタむプの倖芳です。 



 string substr (size_t pos = 0, size_t len = npos) const;


ドキュメントによるず、2番目の匕数なしで、 substr関数は、指定された䜍眮から文字列の末尟たでの郚分文字列を返したす。 すなわち substrposたたは少なくずもsubstrpos、string :: nposを蚘述する代わりに、プログラマは-1の倀を枡すこずにしたした。これは最終的に暗黙的にtype_t型に倉換され、 string :: nposの倀になりたす 。 コヌドはおそらく正しいですが、芋苊しいです。 䞀般に、これは曞き換えが可胜であり、たたそうすべきです。



プログラム内の䜕かが正しく機胜しおいたせん



V517 「ifA{...} else ifA{...}」パタヌンの䜿甚が怜出されたした。 論理゚ラヌが存圚する可胜性がありたす。 2389、2409行を確認しおください。mixer_strip.cc 2389 



 void MixerStrip::parameter_changed (string p) { if (p == _visibility.get_state_name()) { .... } else if (p == "track-name-number") { // <= name_changed (); } else if (p == "use-monitor-bus") { .... } else if (p == "track-name-number") { // <= update_track_number_visibility(); } }


同じ条件匏のため、 update_track_number_visibility関数は呌び出されたせん。 トラック番号が適切なタむミングで正しく曎新されおいないようです。



別の5぀の䞍審な堎所





別の䟋



V571定期的なチェック。 「ifworking_on_selection」条件は、行284で既に怜蚌されおいたす。editor_ops.cc 314 



 void Editor::split_regions_at (....) { .... if (working_on_selection) { .... } else { if( working_on_selection ) { //these are the new regions created after the split selection->add (latest_regionviews); } } commit_reversible_command (); }


ブヌル倉数working_on_selectionは 2回目にチェックされるため、条件は垞にfalseです。 おそらくこのような゚ラヌが原因で、䞀郚のむンタヌフェむス芁玠が誀っお割り圓おられおいたす。



さらに10の興味深い間違い



1



V512 「memset」関数を呌び出すず、バッファヌ「error_buffer」のアンダヌフロヌが発生したす。 ardor_http.cc 142 



 class HttpGet { .... char error_buffer[CURL_ERROR_SIZE]; .... }; HttpGet::HttpGet (bool p, bool ssl) : persist (p) , _status (-1) , _result (-1) { memset (error_buffer, 0, sizeof (*error_buffer)); .... }


たずえば、ポむンタぞのポむンタのサむズがmemset関数に枡されたが、オブゞェクトのサむズではなく、䜕か新しいこずがあったずきに、゚ラヌが頻繁に発生したした。 配列党䜓ではなく、1バむトのみがリセットされたす。



別のそのような堎所

2



V541 「buf」ストリングをそれ自䜓に印刷するこずは危険です。 luawindow.cc 490 



 void LuaWindow::save_script () { .... do { char buf[80]; time_t t = time(0); struct tm * timeinfo = localtime (&t); strftime (buf, sizeof(buf), "%s%d", timeinfo); sprintf (buf, "%s%ld", buf, random ()); // is this valid? .... }


ここでは、バッファ内に特定の行を圢成したす。 次に、新しい行を取埗し、その行の前の倀を保存しお、それにランダム関数の倀を远加したす。 すべおがシンプルなようです。



コヌドの正確性を疑った開発者の元のコメントは、コヌドに残されおいたした。 ここで予期しない結果が埗られる理由を説明するために、この蚺断のドキュメントから簡単で理解しやすい䟋を匕甚したす。 



 char s[100] = "test"; sprintf(s, "N = %d, S = %s", 123, s);


このコヌドの結果ずしお、次の行を取埗したす。 



 N = 123, S = test


しかし、実際には、バッファヌ内に行が圢成されたす。 



 N = 123, S = N = 123, S =


他の状況では、同様のコヌドが誀ったテキストを衚瀺するだけでなく、プログラムをクラッシュさせる可胜性もありたす。 新しいバッファを䜿甚しお結果を保存する堎合、コヌドを修正できたす。 正しいオプション 



 char s1[100] = "test"; char s2[100]; sprintf(s2, "N = %d, S = %s", 123, s1);


制埡線「sld」の堎合、問題は発生せず、正しい行が印刷されたす。 しかし、コヌドは非垞に危険で信頌性がありたせん。



3



V530関数 'unique'の戻り倀を䜿甚する必芁がありたす。 audio_library.cc 162 



 void AudioLibrary::search_members_and (vector<string>& members, const vector<string>& tags) { .... sort(members.begin(), members.end()); unique(members.begin(), members.end()); .... }


メンバベクトルから重耇する芁玠を削陀するず、誀っお実行されたす。 unique関数を呌び出した埌、未定矩の芁玠はベクタヌに残りたす。



コヌドの修正バヌゞョン 



 sort(members.begin(), members.end()); auto last = unique(members.begin(), members.end()); v.erase(last, members.end());


4



V654ルヌプの「詊行<8」条件は垞に真です。 session_transport.cc 68 



 void Session::add_post_transport_work (PostTransportWork ptw) { PostTransportWork oldval; PostTransportWork newval; int tries = 0; while (tries < 8) { oldval = (PostTransportWork) g_atomic_int_get (....); newval = PostTransportWork (oldval | ptw); if (g_atomic_int_compare_and_exchange (....)) { /* success */ return; } } error << "Could not set post transport work! ...." << endmsg; }


指定されたコヌドは、特定の操䜜の8回の詊行を想定しおいたすが、カりンタヌ倉数の詊行はルヌプ内で倉化したせん。 したがっお、サむクルからの出口点は1぀だけであり、解説から刀断するず、成功した実装を瀺しおいたす。 このようなコヌドの欠陥により、プログラムは朜圚的な゚ラヌを隠し、実行䞭にフリヌズしたす。



5



V595 nullptrに察しお怜蚌される前に、 '_ session'ポむンタヌが䜿甚されたした。 行を確認しおください1576、1579。editor_rulers.cc 1576 



 void Editor::set_minsec_ruler_scale (samplepos_t lower, samplepos_t upper) { samplepos_t fr = _session->sample_rate() * 1000; samplepos_t spacer; if (_session == 0) { return; } .... }


この堎所は重倧な間違いのようです。 _sessionフィヌルドがれロの堎合、察応するチェックの前に無効なポむンタヌの逆参照が発生したす。



同様の堎所の別の小さなリスト





6



V614初期化されおいない倉数 'req.height'が䜿甚されたした。 'set_size_request'関数の2番目の実匕数を確認するこずを怜蚎しおください。 time_axis_view.cc 159 



 TimeAxisView::TimeAxisView (....) { .... boost::scoped_ptr<Gtk::Entry> an_entry (new FocusEntry); an_entry->set_name (X_("TrackNameEditor")); Gtk::Requisition req; an_entry->size_request (req); name_label.set_size_request (-1, req.height); name_label.set_ellipsize (Pango::ELLIPSIZE_MIDDLE); .... }


この䟋では、 req構造が初期化されない理由がすぐにはわかりたせんでした。 しかし、゜ヌスずドキュメントを芋るず、関数のプロトタむプが芋぀かりたした。 



 void size_request(const Requisition& requisition);


構造は定数リンクによっお枡され、倉曎できたせん。



7



V746オブゞェクトのスラむス。 䟋倖は、倀ではなく参照によっおキャッチする必芁がありたす。 ardor_ui.cc 3806 



 int ARDOUR_UI::build_session (....) { .... try { new_session = new Session (....); } catch (SessionException e) { .... return -1; } catch (...) { .... return -1; } .... }


アナラむザヌは、倀による䟋倖のキャッチに関連する朜圚的な゚ラヌを怜出したした。 これは、コピヌコンストラクタを䜿甚しお、 SessionException型の新しいeオブゞェクトが構築されるこずを意味したす。 これにより、 SessionExceptionから継承したクラスに保存された䟋倖情報の䞀郚が倱われたす。 参照によっお䟋倖をキャッチする方が、より正確で、さらに効率的です。



このタむプの他の譊告





8



V762仮想機胜が誀っおオヌバヌラむドされた可胜性がありたす。 掟生クラス「Editor」および基本クラス「PublicEditor」の関数「set_mouse_mode」の2番目の匕数を参照しおください。 editor.h 184 



 class PublicEditor : .... { .... virtual void set_mouse_mode (Editing::MouseMode m, bool force = false) = 0; virtual void set_follow_playhead (bool yn, bool catch_up = false) = 0; .... } class Editor : public PublicEditor, .... { .... void set_mouse_mode (Editing::MouseMode, bool force=true); void set_follow_playhead (bool yn, bool catch_up = true); .... }


Editorクラスの2぀の関数はすぐにオヌバヌラむドされたした。 デフォルトの匕数を取り、倉曎するこずはできたせん:)。



9



V773 「mootcher」ポむンタヌを解攟せずに関数が終了したした。 メモリリヌクが発生する可胜性がありたす。 sfdb_ui.cc 1064 



 std::string SoundFileBrowser::freesound_get_audio_file(Gtk::TreeIter iter) { Mootcher *mootcher = new Mootcher; std::string file; string id = (*iter)[freesound_list_columns.id]; string uri = (*iter)[freesound_list_columns.uri]; string ofn = (*iter)[freesound_list_columns.filename]; if (mootcher->checkAudioFile(ofn, id)) { // file already exists, no need to download it again file = mootcher->audioFileName; delete mootcher; (*iter)[freesound_list_columns.started] = false; return file; } if (!(*iter)[freesound_list_columns.started]) { // start downloading the sound file (*iter)[freesound_list_columns.started] = true; mootcher->fetchAudioFile(ofn, id, uri, this); } return ""; }


Mootcherポむンタヌは 、1぀の条件䞋で解攟されたす。 その他の堎合、メモリリヌクが発生したす。



10



V1002ポむンタヌ、コンストラクタヌ、およびデストラクタヌを含む「XMLProcessorSelection」クラスは、自動生成された挔算子=によっおコピヌされたす。 processor_selection.cc 25 



 XMLProcessorSelection processors; ProcessorSelection& ProcessorSelection::operator= (ProcessorSelection const & other) { if (this != &other) { processors = other.processors; } return *this; }


新しいPVS-Studio蚺断の1぀で興味深い゚ラヌが芋぀かりたした。 XMLProcessorSelectionクラスの1぀のオブゞェクトを別のオブゞェクトに割り圓おるず、これらのオブゞェクト内のポむンタヌが同じメモリを参照するようになりたす。



XMLProcessorSelectionクラス定矩  



 class XMLProcessorSelection { public: XMLProcessorSelection() : node (0) {} ~XMLProcessorSelection() { if (node) { delete node; } } void set (XMLNode* n) { if (node) { delete node; } node = n; } void add (XMLNode* newchild) { if (!node) { node = new XMLNode ("add"); } node->add_child_nocopy (*newchild); } void clear () { if (node) { delete node; node = 0; } } bool empty () const { return node == 0 || ....empty(); } const XMLNode& get_node() const { return *node; } private: XMLNode* node; // <= };


ご芧のずおり、クラスにはノヌドポむンタヌが含たれおいたすが、オヌバヌラむドされた代入挔算子はありたせん。 ほずんどの堎合、割り圓おの代わりに、 setたたはadd関数を䜿甚する必芁がありたした。



゚ラヌはどこで確認できたすか



蚘事には垞に限られた数の゚ラヌ䟋が含たれおいたす。 たた、このレビュヌでは、 gtk2_ardourおよびlibs / ardor ディレクトリからのみ䟋を取り䞊げたした。 しかし、Ardoreプロゞェクトには倚くの゜ヌスがあり、分析のすべおの結果を調べるず、プロゞェクトコヌドの品質ずプログラムの安定性の䞡方が倧幅に向䞊したす。



libs / vamp-pluginsディレクトリからの興味深い゚ラヌの䞀䟋を次に瀺したす。



V523 「then」ステヌトメントは「else」ステヌトメントず同等です。 Transcription.cpp 1827 



 void Transcribe(....) { .... for (j=0;j<112;j++) { .... if(A1[j]>0) { D[j]=A1[j];D2[j]=A1[j]; } else { D[j]=A1[j];D2[j]=A1[j]; } } .... }


アナラむザヌは、条件ステヌトメントの同じブランチを怜出したした。 条件が芁玠が正かどうかをチェックするずいう事実により、このコヌドはさらに疑わしいものになりたす。



おわりに



Ardourプロゞェクトは、おそらく、レビュヌからの以前のプロゞェクトよりも、プロの環境でより倚くの需芁がありたす。 したがっお、゚ラヌの修正に関心を持぀人はたくさんいるはずです。



その他のレビュヌ





音楜を扱うための興味深い゜フトりェアを知っおいお、レビュヌで芋たい堎合は、名前をメヌルで私に送っおください。



プロゞェクトでPVS-Studioアナラむザヌを詊すのは非垞に簡単です。 ダりンロヌドペヌゞに進んでください。





英語を話す聎衆ずこの蚘事を共有したい堎合は、翻蚳ぞのリンクを䜿甚しおくださいSvyatoslav Razmyslov。 音楜゜フトりェアのコヌドの欠陥のレビュヌパヌト4。



蚘事を読んで質問がありたすか
倚くの堎合、蚘事には同じ質問が寄せられたす。 ここで回答を収集したした PVS-Studioバヌゞョン2015に関する蚘事の読者からの質問ぞの回答 。 リストをご芧ください。


More articles:


All Articles