移植は微妙な問題です:LinuxでFar Managerをチェックする

Microsoft Windows環境で最も人気のあるファイルマネージャーの1つは、DOS用に作成されたNorton Commanderを引き継いだFar Managerです。 Far Managerは、ファイルシステムの操作(ファイルの作成、編集、表示、コピー、移動、検索、削除)を容易にし、標準機能(ネットワーク、アーカイブ、バックアップなどの操作)を拡張します。 最近、Far ManagerのLinuxへの移植が行われ、これまでにアルファ版がリリースされました。 PVS-Studioチームはこのイベントを無視できなかったため、適合コードの品質を確認することにしました。

写真24






Far Managerについて



Far Manager-Microsoft Windowsファミリーのオペレーティングシステム用のコンソールファイルマネージャーで、キーボードの操作に焦点を当てています。 FAR Managerは、2つのウィンドウのイデオロギー、標準(デフォルト)カラーリング、およびコマンドシステム(キーボードコントロール)を有名なファイルマネージャーNorton Commanderから継承し、ファイルの操作(ファイルとディレクトリの作成、表示、編集、コピー、名前の変更、削除)に便利なユーザーインターフェイスを提供しますなど)。







図1-Windows上のFar Manager 2(画像をクリックして拡大)










図1-Windows上のFar Manager 2(画像をクリックして拡大)



プログラムの作者はユージンロシャルです。 最初のバージョンは1996年9月10日にリリースされました。 Roshalが手がけた最新バージョンは、2000年6月23日(バージョン1.65)に遡ります。 実際、その瞬間から、FAR GroupはFAR Managerの開発を開始しました。 次のリリースv1.70は、2006年3月29日にさかのぼります。 2008年12月13日、バージョン2.0がリリースされ、プログラムは無料(オープンソース)になり、修正BSDライセンスの下で配布されました。 1.70から2.0までのすべてのFarバージョンには、実質的に外部の違いはなく、新しいバージョンに切り替えるときにユーザーがプログラムをマスターするための追加の作業は必要ありません。 バージョン1.80から、Unicodeサポートが登場しました。 最新リリースバージョンは、2016年11月4日から3.0です。



2016年8月10日に、 Far2lファイルマネージャー(Linux)の最初のテストビルドが公開されました。 現時点では、アセンブリには統合作業端末と、Align、AutoWrap、Colorer、DrawLine、Editcase、FarFTP、FarLng、MultiArc、NetBox、SimpleIndent、TmpPanelのプラグインが含まれています。 コードはGPLv2の下でライセンスされています。







図2-Linux上のFar Manager 2(画像をクリックして拡大)










図2-Linux上のFar Manager 2(画像をクリックして拡大)



言葉から行為へ



Far2lプロジェクトの検証後、1,038件の一般的な警告が受信されました。 次の図は、信頼レベルごとの警告の分布を示しています。







図1-信頼レベルによる警告の分布(重要度)










図1-信頼レベルによる警告の分布(重要度)



以下の図について簡単にコメントしましょう。153の高レベルの警告、336の中レベルの警告、549の低レベルの警告が受信されました。



かなりの数の警告にもかかわらず、すべての警告が本当の間違いではないことを覚えておく価値があります。 高レベルおよび中レベルの警告のみを含むレポートを確認した後、250件のケースを強調表示しました。これは、本当のエラーである可能性が高いです。



高レベルと中レベルを選択すると、誤検知の割合は約49%でした。 言い換えれば、1秒おきの警告はコードの欠陥を特定します。



ここで、PVS-Studioアナライザーによって検出された実際のエラーの相対密度を決定します。 ソースコード(SLOC)の合計行数は538675です。したがって、密度はコード1000行あたり0.464エラーに等しくなります。 いつかこれらの数値を収集し、さまざまなプロジェクトのコードの品質に関する一般的な記事を書きます。



このインジケータはプロジェクト全体の合計エラー密度を示していないことは注目に値します-大きい(アナライザーが実際のエラーで動作しなかった)か、小さい(アナライザーが正しいコードで動作した)ことがあります。 原則として、他のプロジェクトでは検出されるエラーの密度が高くなります。 コード品質の観点から、移植は成功したと言えます。 ただし、見つかったエラーは無害ではないため、修正することを強くお勧めします。



検証結果



読みやすくするために、コードがリファクタリングされることを事前に警告します。 また、この記事には検証中に見つかったすべてのエラーが含まれているわけではなく、最も興味深いエラーのみが含まれています。



コピーペースト







写真28










アナライザーの警告V501 「||」の左側と右側に同一の副次式「Key == MCODE_F_BM_GET」があります 演算子。 macro.cpp 4819



int KeyMacro::GetKey() { .... DWORD Key = !MR ? MCODE_OP_EXIT : GetOpCode(MR, Work.ExecLIBPos++); .... switch (Key) { .... case MCODE_F_BM_POP: { TVar p1, p2; if (Key == MCODE_F_BM_GET) VMStack.Pop(p2); if ( Key == MCODE_F_BM_GET // <= || Key == MCODE_F_BM_DEL || Key == MCODE_F_BM_GET // <= || Key == MCODE_F_BM_GOTO) { VMStack.Pop(p1); } .... } } }
      
      





Key変数は、定数MCODE_F_BM_GETと2回比較されました。 これはおそらくタイプミスであり、 Keyは他の定数と比較されるべきでした。 アナライザーは、さらに3つの類似した場所を見つけました。





アナライザーの警告V581互いに並んでいる「if」演算子の条件式は同一です。 行を確認:267、268。APIStringMap.cpp 268



 static BOOL WINPORT(GetStringType)( DWORD type, LPCWSTR src, INT count, LPWORD chartype ) { .... while (count--) { int c = *src; WORD type1, type3 = 0; /* C3_NOTAPPLICABLE */ .... if ((c>=0xFFE0)&&(c<=0xFFE6)) type3 |= C3_FULLWIDTH; // <= if ((c>=0xFFE0)&&(c<=0xFFE6)) type3 |= C3_SYMBOL; // <= .... } .... }
      
      





どうやら、2番目の条件はコピーペーストの原則に従って記述されたもので、最初の条件とまったく同じです。 ただし、意図がこれである場合は、2番目の条件を削除することでコードを簡素化できます。



 .... if ((c>=0xFFE0)&&(c<=0xFFE6)) type3 |= C3_FULLWIDTH | C3_SYMBOL; ....
      
      





見つかったエラーは、唯一のものではありませんでした。





アナライザーの警告V523 「then」ステートメントは「else」ステートメントと同等です。 Queque.cpp 358



 void FTP::AddToQueque(FAR_FIND_DATA* FileName, LPCSTR Path, BOOL Download) { .... char *m; .... if(Download) m = strrchr(FileName->cFileName, '/'); // <= else m = strrchr(FileName->cFileName, '/'); // <= .... }
      
      





ここでは、条件が「コピー-貼り付け」の原則に従って書き込まれたと思われます。 ダウンロードの値( TRUEFALSE )に関係なく、「/」文字の最後の出現位置はmポインターに保存されます。



未定義の動作







写真37










アナライザーの警告V567未定義の動作。 'Item [FocusPos]-> Selected'変数は、シーケンスポイント間で2回使用されている間に変更されます。 dialog.cpp 3827



 int Dialog::Do_ProcessSpace() { .... if (Item[FocusPos]->Flags & DIF_3STATE) (++Item[FocusPos]->Selected) %= 3; // <= else Item[FocusPos]->Selected = !Item[FocusPos]->Selected; .... }
      
      





ここには明確な未定義の動作があります: アイテム[FocusPos]->選択された変数同じポイントで2回変更さます(結果の割り当てで3を法とするインクリメントと除算)。



同様の未定義の動作を持つ別の場所が見つかりました:





アナライザーの警告 :V610未定義の動作。 シフト演算子「<<」を確認してください。 右側のオペランド 'sizeof(wchar_t)* 8'は、昇格した左側のオペランドのビット単位の長さ以上です。 RegExp.cpp 4467



 #define rechar wchar_t #define RE_CHAR_COUNT (1 << sizeof(rechar) * 8) int RegExp::Optimize() { .... for (op=code; ; op=op->next) { switch (OP.op) { .... case opType: { for (int i = 0; i < RE_CHAR_COUNT; i++) // <= { if (ISTYPE(i, OP.type)) { first[i]=1; } } break; } } .... } .... }
      
      





エラーの本質は次のとおりです。Linuxでは、 wchar_t型のサイズは4バイトです。 したがって、signed int (4バイト)は32ビット左にシフトされます。 C ++ 11標準によれば、左のオペランドが符号付きの正の数である場合、Nが左のオペランドのビット数以上の場合、Nビットだけ左にシフトすると未定義の動作になります。 正しいコードは次のようになります。



 #define rechar wchar_t #define RE_CHAR_COUNT (static_cast<int64_t>(1) << sizeof(rechar) * 8) int RegExp::Optimize() { .... for (int64_t i = 0; i < RE_CHAR_COUNT; i++) { .... } .... }
      
      





左にシフトするとき、未定義の動作につながる場所がさらにいくつか見つかりました。





誤ったメモリ処理







写真41












ちょっとしたトレーニングで新しいセクションを始めましょう。 自分でエラーを見つけることをお勧めします(ヒントはTreeItem :: SetTitle関数にあります )。



 class UnicodeString { .... UnicodeString(const wchar_t *lpwszData) { SetEUS(); Copy(lpwszData); } .... const wchar_t *CPtr() const { return m_pData->GetData(); } operator const wchar_t *() const { return m_pData->GetData(); } .... } typedef UnicodeString FARString; struct TreeItem { FARString strName; .... } TreeItem **ListData;
      
      



 void TreeList::SetTitle() { .... if (GetFocus()) { FARString strTitleDir(L"{"); const wchar_t *Ptr = ListData ? ListData[CurFile]->strName : L""; .... } .... }
      
      





アナライザーの警告V623 「?:」演算子の検査を検討してください。 「UnicodeString」タイプの一時オブジェクトが作成され、その後破棄されます。 第3オペランドを確認してください。 treelist.cpp 2093



明らかな間違いではありませんか? 現在のコンテキストでは、変数ListData [CurFile]-> strNameUnicodeStringクラスのオブジェクトです。 UnicodeStringクラスでは、 constwchar_t *への暗黙的な変換演算子オーバーロードされます。 次に、 TreeList :: SetTitle関数の三項演算子に注意してください。2番目と3番目のオペランドは異なるタイプです(それぞれUnicodeStringconst char [1] )。 最初のオペランドがfalseを返す場合、 Ptrポインターは空の文字列にアドレス指定されることが理解されました。 UnicodeStringクラスのコンストラクタは明示的に宣言されていないため、実際には、空の文字列を含む一時オブジェクトが作成され、 const wchar_t *に暗黙的にキャストされます。 その後、一時オブジェクトは破棄され、 Ptrは無効なデータを示します。 修正されたコードは次のようになります。



 .... const wchar_t *Ptr = ListData ? ListData[CurFile]->strName.CPtr() : L""; ....
      
      





次のコードは、2つの診断が同時に機能したことで注目に値します。



アナライザーの警告





 BOOL WINAPI _export SEVENZ_OpenArchive(const char *Name, int *Type) { Traverser *t = new Traverser(Name); if (!t->Valid()) { return FALSE; delete t; } delete s_selected_traverser; s_selected_traverser = t; return TRUE; }
      
      





ここで何が見つかりますか? まず、実際には、 ifステートメントに到達できないコードがあります。条件が満たされると、関数はFALSEを返し、作業を完了します 。 また、到達不能なコードのため、メモリリークのみが発生しました。ポインタtのオブジェクトは削除されません。 エラーを修正するには、ブロック内の2つの演算子を交換します。



次のコードは、ポインターを介してクラス(構造)のオブジェクトのサイズを決定するときに、どのように間違えるかを示しています。



アナライザーの警告





 int64_t FileList::VMProcess(int OpCode, void *vParam, int64_t iParam) { switch (OpCode) { .... case MCODE_V_PPANEL_PREFIX: // PPanel.Prefix { PluginInfo *PInfo = (PluginInfo *)vParam; memset(PInfo, 0, sizeof(PInfo)); // <= PInfo->StructSize = sizeof(PInfo); // <= .... } .... } }
      
      





両方のエラーは、構造体の予想サイズではなく、 sizeof(PInfo)がポインターのサイズ(4または8バイト)を返すことです。 したがって、 memsetは構造体の最初の4(8)バイトのみをゼロで埋めます。また、ポインターのサイズはPInfo-> StructSizeフィールドに書き込まれます。 正しいコードは次のようになります。



 .... PluginInfo *PInfo = (PluginInfo*)vParam; memset(PInfo, 0, sizeof(*PInfo)); PInfo->StructSize = sizeof(*PInfo); ....
      
      





アナライザーは、さらにいくつかの類似した場所を見つけました。





奇妙な条件







写真39










繰り返しになりますが、少しウォームアップします。コードの次の部分で自分でエラーを見つけてください。



 int FTP::ProcessKey(int Key, unsigned int ControlState) { .... if( !ShowHosts && (ControlState == 0 || ControlState == PKF_SHIFT) && Key == VK_F6) { FTP *ftp = OtherPlugin(this); int rc; if( !ftp && ControlState == 0 && Key == VK_F6) { return FALSE; } .... } .... }
      
      





アナライザーの警告V560条件式の一部は常にtrueです:キー== 0x75。 Key.cpp 493



外部および内部の条件に注意してください 。それらのキーは定数VK_F6と比較されます。 制御フローが内部条件に達すると、 Key変数はVK_F6と等しくなることが保証され、この変数の2回目のチェックは不要になります。 簡略化された形式では、2番目の条件は次のようになります。



 .... if( !ftp && ControlState == 0) { return FALSE; } ....
      
      





アナライザーは、このエラーといくつかの同様のエラーについて警告します。





アナライザーの警告V503これは無意味な比較です:ポインター<=0。fstd_exSCPY.cpp 8



 char *WINAPI StrCpy(char *dest, LPCSTR src, int dest_sz) { if(dest <= 0) // <= return NULL; .... }
      
      





このコードには、ポインターと負の数の無意味な比較が含まれています(ポインターは負のアドレスを持つメモリ領域では機能しません)。 修正されたコードは次のようになります。



 .... if(dest == nullptr) return NULL; ....
      
      





アナライザーの警告V584 FADC_ALLDISKS値は、「==」演算子の両側にあります。 式が正しくないか、単純化できます。 findfile.cpp 3116



 enum FINDASKDLGCOMBO { FADC_ALLDISKS, FADC_ALLBUTNET, .... }; FindFiles::FindFiles() { .... if ( FADC_ALLDISKS + SearchMode == FADC_ALLDISKS // <= || FADC_ALLDISKS + SearchMode == FADC_ALLBUTNET) { .... } .... }
      
      





アナライザーは、疑わしい最初の副条件を検出しました。 FINDASKDLGCOMBO列挙に基づいて、定数FADC_ALLDISKSは0、 FADC_ALLBUTNETは1です。条件式で定数の数値を代入すると、次のようになります。



 if ( 0 + SearchMode == 0 || 0 + SearchMode == 1) { .... }
      
      





上記のコードに基づいて、条件全体を単純化できます。



 if ( SearchMode == FADC_ALLDISKS || SearchMode == FADC_ALLBUTNET) { .... }
      
      





誤ったフォーマット文字列操作







写真40










アナライザーの警告V576の形式が正しくありません 。 'swprintf'関数の4番目の実引数を確認することを検討してください。 char型の引数が必要です。 FarEditor.cpp 827



 void FarEditor::showOutliner(Outliner *outliner) { .... wchar_t cls = Character::toLowerCase((*region)[region->indexOf(':') + 1]); si += swprintf(menuItem+si, 255-si, L"%c ", cls); // <= .... }
      
      





これはおそらく移植エラーです。 問題は、Visual C ++では、ワイドライン出力関数がフォーマット文字列の指定子を型破りに解釈することです。 %cは、ワイド文字(ワイド文字、 wchar_t )を期待します。 Linuxでは、状況は異なります。 %c指定子の標準に従って、マルチバイト文字( char )が期待されます。 正しいコードは次のようになります。



 si += swprintf(menuItem+si, 255-si, L"%lc ", cls);
      
      





アナライザーの警告V576の形式が正しくありません 。 'swprintf'関数の4番目の実引数を確認することを検討してください。 char型のシンボルの文字列へのポインタが必要です。 cmddata.cpp 257



 void CommandData::ReadConfig() { .... wchar Cmd[16]; .... wchar SwName[16+ASIZE(Cmd)]; swprintf(SwName,ASIZE(SwName), L"switches_%s=", Cmd); // <= .... }
      
      





同様の状況:フォーマット文字列には%s指定子が含まれるため、マルチバイト文字列( char * )が期待されます。 ただし、次のパラメーターはワイド文字列( wchar_t * )を渡しました。 正しいコードは次のようになります。



 swprintf(SwName,ASIZE(SwName), L"switches_%ls=", Cmd);
      
      





アナライザーは、フォーマット文字列に従ってパラメーターを渡す他の2つの誤った方法についても警告します。





結論



LinuxのFarポートはどうですか? はい、十分なエラーが見つかりましたが、プロジェクトがアルファ版であり、開発を続けていることを忘れないでください。 静的コード分析の方法論を使用すると、エラーは早い段階で検出され、リポジトリに入れられません。 静的解析のすべての利点は、通常の使用(極端な場合-「夜間」のアセンブリ中)でのみ明らかになることに注意してください。



私に代わって、PVS-Studioを使用した静的解析の利点を評価することを提案します。この製品はMicrosoft Windowsおよびdeb / rpmベースのLinuxディストリビューションで利用でき、プロジェクトを迅速かつ定期的に確認できます。 また、あなたが学生であるか、個々の開発者であるか、オープンな非営利プロジェクトの開発に関与している場合、PVS-Studioを無料で使用できる可能性があります。



この入門ビデオでは、PVS-Studio for Linuxをインストールし、プロジェクトをすばやく確認する方法を学習できます(例としてFar Managerを使用)。







また、チェックすべき興味深いプロジェクトを知っている場合は、 GitHubで提供できます。 詳細については、「 PVS-Studioアナライザーを使用したテスト用プロジェクトの提供:GitHubでの提供 」をご覧ください。







この記事を英語圏の聴衆と共有したい場合は、翻訳リンクを使用してください:フィリップ・カンデリアンツ。 移植はデリケートな問題です:LinuxでのFar Managerの確認



記事を読んで質問がありますか?
多くの場合、記事には同じ質問が寄せられます。 ここで回答を収集しました: PVS-Studioバージョン2015に関する記事の読者からの質問への回答 。 リストをご覧ください。



All Articles