CGrepAgent::DoGrep のローカル変数初期化漏れ修正 #236

yoshinrt · 2018-07-08T11:40:39Z

#225 の発端となった初期化漏れの修正です．
「ラインモード貼付けを可能にする」off 時は bLineSelect = false であるべきなので，そのように初期化．

ちなみ初期化の方法は↓に合わせています．
(どちらも，ダイアログボックスのテキストボックスにクリップボードのデータを貼り付ける動作)

sakura/sakura_core/cmd/CViewCommander_Search.cpp

Lines 868 to 875 in 5da6469

    
           bool		bLineSelect = false;	// ラインモード貼り付けを行うかどうか 
        
           CNativeW	cmemClip;				// 置換後文字列のデータ（データを格納するだけで、ループ内ではこの形ではデータを扱いません）。 
        
           // クリップボードからのデータ貼り付けかどうか。 
        
           if( nPaste != 0 ) 
        
           { 
        
           	// クリップボードからデータを取得。 
        
           	if ( !m_pCommanderView->MyGetClipboardData( cmemClip, &bColumnSelect, GetDllShareData().m_Common.m_sEdit.m_bEnableLineModePaste? &bLineSelect: NULL ) )

yoshinrt · 2018-07-08T11:45:34Z

他の C4701 については↓以外は自分目線で問題なさそう，程度しか見ていませんので，#225 を残す・閉じるは他の方の判断におまかせしたいと思います．

d:\dds\sakura\sakura_core\cmd\cviewcommander.cpp(671): warning C4701: 初期化されていない可能性のあるローカル変数 'bSelLock' が使用されます [D:\DDS\sakura\sakura\sakura.vcxproj]

berryzplus

PRありがとうございます。
LGTMです。

Grep置換で「クリップボードから貼り付け」にして動かしてみましたが動作にも影響していないようです。

berryzplus · 2018-07-08T12:22:27Z

sakura_core/CGrepAgent.cpp

@@ -231,7 +231,7 @@ DWORD CGrepAgent::DoGrep(
 		if( bGrepPaste ){
 			// ��`�E���C�����[�h�\��t���͖��T�|�[�g
 			bool bColmnSelect;
-			bool bLineSelect;
+			bool bLineSelect = false;
 			if( !pcViewDst->MyGetClipboardData( cmemReplace, &bColmnSelect, GetDllShareData().m_Common.m_sEdit.m_bEnableLineModePaste? &bLineSelect: NULL ) ){


参考ですが、このbLineSelectは、SALで書くと _Out_writes_opt_ bool* bLineSelect に渡すパラメータです。平たく言うと「出力パラメータなので初期化しなくてもいい」です。

呼び出されるメソッドの中

sakura/sakura_core/view/CEditView.cpp

Lines 2394 to 2400 in 5da6469

bool CEditView::MyGetClipboardData( CNativeW& cmemBuf, bool* pbColumnSelect, bool* pbLineSelect /*= NULL*/ )

{

if(pbColumnSelect)

*pbColumnSelect = false;

if(pbLineSelect)

*pbLineSelect = false;

さらに呼び出されるメソッド

sakura/sakura_core/_os/CClipboard.cpp

Lines 269 to 282 in 5da6469

bool CClipboard::GetText(CNativeW* cmemBuf, bool* pbColumnSelect, bool* pbLineSelect, const CEol& cEol, UINT uGetFormat)

{

if( !m_bOpenResult ){

return false;

}

if( NULL != pbColumnSelect ){

*pbColumnSelect = false;

}

if( NULL != pbLineSelect ){

*pbLineSelect = false;

}

//矩形選択や行選択のデータがあれば取得

if( NULL != pbColumnSelect || NULL != pbLineSelect ){

個人的に、初期化漏れパラメータはすっごく気になるんですけど、こういうこともあるようです。

参考ですが、このbLineSelectは、SALで書くと Out_writes_opt bool* bLineSelect に渡すパラメータです。平たく言うと「出力パラメータなので初期化しなくてもいい」です。

GetDllShareData().m_Common.m_sEdit.m_bEnableLineModePaste が FALSE の場合は
&bLineSelect が渡されないので初期化されないと思います。

m-tmatma · 2018-07-10T13:53:33Z

テスト方法を共有していただけますか?

berryzplus · 2018-07-10T14:23:17Z

GetDllShareData().m_Common.m_sEdit.m_bEnableLineModePaste が FALSE の場合は
&bLineSelect が渡されないので初期化されないと思います。

その通りですね。
Releaseビルドでテストしたので普通に通ったようです。

テスト方法を共有していただけますか?

	if( bGrepReplace ){ //←Grep置換ダイアログで「置換」
		if( bGrepPaste ){ //←Grep置換ダイアログで「クリップボードから貼り付ける」にチェック

GetDllShareData().m_Common.m_sEdit.m_bEnableLineModePaste の設定は
共通設定 > 編集タブ
「ラインモード貼り付けを可能にする」のON/OFFで切り替わる

クリップボードに入っているデータの種類によって動作を切り替えるかどうかの設定です。

vs2017もしくはサクラエディタで、データを選択せずにコピーする。
vs2017もしくはサクラエディタで、カーソルを行の途中（行頭でも行末でもない位置）において貼り付けるとカーソル行の手前にコピー時にカーソルがあった行の全体が貼り付けされる。通常モードではカーソル位置にデータが貼り付く。これがラインモード貼り付け。
Grep置換ダイアログを開いて「クリップボードから貼り付ける」をチェックして「置換」を実行。
ファイルが置換されてしまうので対象フォルダにはダミーデータを用意しておく。
設定に応じて「ラインモード貼り付け」の有効・無効が切り替わるか確認する。
ラインモード有効なら検索一致行の前にコピーした行が挿入される。
ラインモード無効なら検索一致文字列が置換される。

ぼくが確認したのは変更前後のリリース版の挙動。

テストしないといけないのは変更前後のデバッグ版の挙動。
デバッグ版だとラインモード無効時の挙動が変わるはず。
変更前はラインモード有効になり、変更後は設定通り無効になる。

デバッグ版だとアサーションで止まるかも、ということに書いてから気付いた。

yoshinrt · 2018-07-10T15:47:09Z

@berryzplus さん，テスト方法の共有ありがとうございます．
デバッグビルドだと，アサーション発火しました．

ラインモード有効なら検索一致行の前にコピーした行が挿入される。

自分で直しといてなんですが，置換時におけるこの仕様って需要あるんでしょうか...
m_bEnableLineModePaste にかかわらずラインモード貼付けは無し，が自分的にはしっくりきます．

kobake · 2018-07-11T08:18:24Z

master 時でのアサーション発火、こちらでも確認。

本 PR 摘要後にはこれは出なくなっています。

kobake

対応内容については LGTM です

機能そのものの需要とか正当性とかについては別途議論ですかね～

…var_init CGrepAgent::DoGrep のローカル変数初期化漏れ修正

CGrepAgent::DoGrep のローカル変数 (bLineSelect) 初期化漏れ

d3075c2

berryzplus approved these changes Jul 8, 2018

View reviewed changes

m-tmatma added this to the next release milestone Jul 10, 2018

m-tmatma added the 🐛bug🦋 ■バグ修正(Something isn't working) label Jul 10, 2018

kobake approved these changes Jul 11, 2018

View reviewed changes

kobake merged commit 7e668d2 into sakura-editor:master Jul 11, 2018

yoshinrt deleted the fixup_dogrep_localvar_init branch July 11, 2018 11:25

ds14050 added the 🐛bug🦋 ■バグ修正(Something isn't working) label Sep 18, 2018

HoppingTappy pushed a commit to HoppingTappy/sakura that referenced this pull request Jun 11, 2019

Merge pull request sakura-editor#236 from yoshinrt/fixup_dogrep_local…

ffe2a29

…var_init CGrepAgent::DoGrep のローカル変数初期化漏れ修正

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CGrepAgent::DoGrep のローカル変数初期化漏れ修正 #236

CGrepAgent::DoGrep のローカル変数初期化漏れ修正 #236

yoshinrt commented Jul 8, 2018

yoshinrt commented Jul 8, 2018

berryzplus left a comment

berryzplus Jul 8, 2018 •

edited

Loading

m-tmatma Jul 10, 2018

m-tmatma commented Jul 10, 2018

berryzplus commented Jul 10, 2018 •

edited

Loading

yoshinrt commented Jul 10, 2018

kobake commented Jul 11, 2018

kobake left a comment

	bool bLineSelect = false; // ラインモード貼り付けを行うかどうか
	CNativeW cmemClip; // 置換後文字列のデータ（データを格納するだけで、ループ内ではこの形ではデータを扱いません）。

	// クリップボードからのデータ貼り付けかどうか。
	if( nPaste != 0 )
	{
	// クリップボードからデータを取得。
	if ( !m_pCommanderView->MyGetClipboardData( cmemClip, &bColumnSelect, GetDllShareData().m_Common.m_sEdit.m_bEnableLineModePaste? &bLineSelect: NULL ) )

	bool CEditView::MyGetClipboardData( CNativeW& cmemBuf, bool* pbColumnSelect, bool* pbLineSelect /= NULL/ )
	{
	if(pbColumnSelect)
	*pbColumnSelect = false;

	if(pbLineSelect)
	*pbLineSelect = false;

	bool CClipboard::GetText(CNativeW* cmemBuf, bool* pbColumnSelect, bool* pbLineSelect, const CEol& cEol, UINT uGetFormat)
	{
	if( !m_bOpenResult ){
	return false;
	}
	if( NULL != pbColumnSelect ){
	*pbColumnSelect = false;
	}
	if( NULL != pbLineSelect ){
	*pbLineSelect = false;
	}

	//矩形選択や行選択のデータがあれば取得
	if( NULL != pbColumnSelect \|\| NULL != pbLineSelect ){

CGrepAgent::DoGrep のローカル変数初期化漏れ修正 #236

CGrepAgent::DoGrep のローカル変数初期化漏れ修正 #236

Conversation

yoshinrt commented Jul 8, 2018

yoshinrt commented Jul 8, 2018

berryzplus left a comment

Choose a reason for hiding this comment

berryzplus Jul 8, 2018 • edited Loading

Choose a reason for hiding this comment

m-tmatma Jul 10, 2018

Choose a reason for hiding this comment

m-tmatma commented Jul 10, 2018

berryzplus commented Jul 10, 2018 • edited Loading

yoshinrt commented Jul 10, 2018

kobake commented Jul 11, 2018

kobake left a comment

Choose a reason for hiding this comment

berryzplus Jul 8, 2018 •

edited

Loading

berryzplus commented Jul 10, 2018 •

edited

Loading