Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

引数の文字列コピーに検出したnArgLenを使う #1427

Merged

Conversation

berryzplus
Copy link
Contributor

PR の目的

-GKEY などの文字列を指定できるコマンドラインオプションを内部変数にコピーする処理で、検出した文字列長を使わずに再計算しているのが効率悪いので、検出済みの文字列長を使うように修正します。

カテゴリ

  • リファクタリング

PR の背景

コマンドラインオプションには、文字列値をパラメータとして指定できるものがあります。

たとえば -GKEY の設定はこうなっています。

case CMDLINEOPT_GKEY: // GKEY
// 前後の""を取り除く
m_gi.cmGrepKey.SetString( arg, lstrlen( arg ) );
m_gi.cmGrepKey.Replace( L"\"\"", L"\"" );
break;

このPRでやりたいことは lstrlen( arg ) をやめることです。

argの宣言はこうなっていて、nArgLenとセットで宣言されています。

WCHAR *arg = NULL;
int nArgLen;
switch( CheckCommandLine( pszToken, &arg, &nArgLen ) ){

CCommandLine::CheckCommandLine は、オプション文字列 -GKEY="\s+" を受け取って \s+ を検出する関数です。

int CCommandLine::CheckCommandLine(
LPWSTR str, //!< [in] 検証する文字列(先頭の-は含まない)
WCHAR** arg, //!< [out] 引数がある場合はその先頭へのポインタ
int* arglen //!< [out] 引数の長さ
)

検出した文字列長があるのにそれを使わず、lstrlen したり省略して wcslen したりするのは無駄だと思います。

PR のメリット

CCommandLine::CheckCommandLine で検出した文字列長を使うことにより、不要な文字列長計算を行わなくてすむようになります。

PR のデメリット (トレードオフとかあれば)

CCommandLine::CheckCommandLine の文字列長検出に誤りがあった場合、文字列値を指定できるコマンドラインオプションの解析結果に悪影響が出るようになります。

仕様・動作説明

仕様・動作に変更はありません。

テスト内容

変更行は全行が単体テスト https://github.com/sakura-editor/sakura/blob/master/tests/unittests/test-ccommandline.cpp の範囲に含まれるため、追加で実施するテストはありません。

PR の影響範囲

文字列値を指定できるコマンドラインオプションを指定したときの動作に影響します。

関連 issue, PR

#1420

参考資料

@AppVeyorBot
Copy link

@beru beru added the refactoring リファクタリング 【ChangeLog除外】 label Oct 14, 2020
Copy link
Contributor

@beru beru left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

問題ないと思います。

case CMDLINEOPT_AT: では既にその方法が取られていますね。

@berryzplus
Copy link
Contributor Author

レビューありがとうございます。マージしちゃいます。

@berryzplus berryzplus merged commit 35d0498 into sakura-editor:master Oct 14, 2020
@berryzplus berryzplus deleted the feature/use_detected_arg_length branch October 14, 2020 16:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactoring リファクタリング 【ChangeLog除外】
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants