Merge pull request #2191 from berryzplus/feature/omit_is_bad_string_ptr

IsBadStringPtrAの呼出を除去する

Author: berryzplus

sakura_core/_main/global.cpp

@@ -18,6 +18,7 @@
 #include "StdAfx.h"
 #include "_main/global.h"
 
+#include "_main/CCommandLine.h"
 #include "_main/CNormalProcess.h"
 #include "basis/CErrorInfo.h"
 #include "config/app_constants.h"
@@ -55,6 +56,15 @@ HINSTANCE G_AppInstance()
 	return CProcess::getInstance()->GetProcessInstance();
 }
 
+/*!
+ * 起動時に指定されたプロファイル名を取得する
+ */
+LPCWSTR GetProfileName() noexcept
+{
+	const auto cmdline = CCommandLine::getInstance();
+	return cmdline ? cmdline->GetProfileName() : L"";
+}
+
 /*!
  * コンストラクタ
  */

sakura_core/_main/global.h

@@ -170,4 +170,7 @@ struct SSearchOption{
 };
 
 HINSTANCE G_AppInstance();
+
+LPCWSTR		GetProfileName() noexcept;
+
 #endif /* SAKURA_GLOBAL_BD124BE7_A850_41F3_9F5D_8A4F0E45A4C9_H_ */

sakura_core/env/CShareData.cpp

@@ -113,7 +113,7 @@ bool CShareData::InitShareData()
 
 	/* ファイルマッピングオブジェクト */
 	{
-		const auto pszProfileName = CCommandLine::getInstance()->GetProfileName();
+		const auto pszProfileName = GetProfileName();
 		std::wstring strShareDataName = GSTR_SHAREDATA;
 		strShareDataName += pszProfileName;
 		m_hFileMap = ::CreateFileMapping(

sakura_core/macro/CPPA.cpp

@@ -299,6 +299,24 @@ void __stdcall CPPA::stdStrObj(const char* ObjName, int Index, BYTE GS_Mode, int
 	}
 }
 
+/*!
+ * CPPAエラー情報コールバックをテストできるようI/Fを公開する関数。
+ *
+ * 既存コードをできるだけ改変せずにテストできるよう作成。
+ * 可能であれば、そのうち消す。
+ */
+/* static */ void CPPA::CallErrorProc(PpaExecInfo& info, int Err_CD, _In_opt_z_ LPCSTR Err_Mes)
+{
+	// PPA実行情報をセットする
+	m_CurInstance = &info;
+
+	// CPPAエラー情報コールバックを呼び出す
+	stdError(Err_CD, Err_Mes);
+
+	// PPA実行情報をクリアする
+	m_CurInstance = nullptr;
+}
+
 /*! ユーザー定義関数のエラーメッセージの作成
 
 	stdProc, stdIntFunc, stdStrFunc がエラーコードを返した場合、PPAから呼び出される。
@@ -346,9 +364,8 @@ void __stdcall CPPA::stdError( int Err_CD, const char* Err_Mes )
 			auto_sprintf( szMes, LS(STR_ERR_DLGPPA3), FuncID );
 		}
 	}else{
-		//	2007.07.26 genta : ネスト実行した場合にPPAが不正なポインタを渡す可能性を考慮．
-		//	実際には不正なエラーは全てPPA.DLL内部でトラップされるようだが念のため．
-		if( IsBadStringPtrA( Err_Mes, 256 )){
+		if( !Err_Mes ){
+			// "エラー情報が不正"
 			pszErr = LS(STR_ERR_DLGPPA6);
 		}else{
 			switch( Err_CD ){

sakura_core/macro/CPPA.h

@@ -252,21 +252,24 @@ class CPPA : public CDllImp {
 	//	メンバ変数
 	char		m_szMsg[80];		//!< CPPAからのメッセージを保持する
 
+public:
 	//	2007.07.26 genta : PPAのネストを許容するために，別データ構造とする．
 
 	struct PpaExecInfo {
-		CNativeA		m_cMemRet;		//!< コールバックからDLLに渡す文字列を保持
-		CEditView*		m_pcEditView;	//	2003.06.01 Moca
-		DLLSHAREDATA*	m_pShareData;	//	2003.06.01 Moca
-		bool			m_bError;		//!< エラーが2回表示されるのを防ぐ	2003.06.01 Moca
-		CNativeA		m_cMemDebug;	//!< デバッグ用変数UserErrorMes 2003.06.01 Moca
+		CNativeA		m_cMemRet = "";			//!< コールバックからDLLに渡す文字列を保持
+		CEditView*		m_pcEditView = nullptr;	//	2003.06.01 Moca
+		DLLSHAREDATA*	m_pShareData = nullptr;	//	2003.06.01 Moca
+		bool			m_bError = false;		//!< エラーが2回表示されるのを防ぐ	2003.06.01 Moca
+		CNativeA		m_cMemDebug = "";		//!< デバッグ用変数UserErrorMes 2003.06.01 Moca
 		/** オプションフラグ
 		
 			CEditView::HandleCommand()にコマンドと一緒に渡すことで
 			コマンドの素性を教える．
 		*/
-		int				m_commandflags;	//!< 
+		int				m_commandflags = 0;
 	};
+
+private:
 	//	2007.07.26 genta : 現在実行中のインスタンス
 	static PpaExecInfo* m_CurInstance;
 	//PPAの多重起動禁止 2008.10.22 syat
@@ -276,5 +279,9 @@ class CPPA : public CDllImp {
 	static struct MacroFuncInfo	S_Table[];
 	static int					m_nFuncNum;	//	SAKURAエディタ用関数の数
 */
+
+public:
+	static void CallErrorProc(PpaExecInfo& info, int Err_CD, _In_opt_z_ LPCSTR Err_Mes);
 };
+
 #endif /* SAKURA_CPPA_FB41BBAE_DFBC_449D_9342_5D9424CFE086_H_ */

sakura_core/util/relation_tool.cpp

@@ -11,38 +11,53 @@
 //                         CSubject                            //
 // -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- //
 
-CSubject::CSubject()
-{
-}
+CSubject::CSubject() = default;
 
 CSubject::~CSubject()
 {
 	//リスナを解除
-	for(int i=0;i<(int)m_vListenersRef.size();i++){
-		m_vListenersRef[i]->Listen(nullptr);
+	while (!m_vListenersRef.empty()) {
+		_RemoveListener(m_vListenersRef.back());
 	}
-	m_vListenersRef.clear();
 }
 
+/*!
+ * リスナーを追加する
+ */
 void CSubject::_AddListener(CListener* pcListener)
 {
+	//NULLは追加できない
+	assert(pcListener);
+
 	//既に追加済みなら何もしない
-	for(int i=0;i<(int)m_vListenersRef.size();i++){
-		if(m_vListenersRef[i]==pcListener){
-			return;
-		}
+	if (auto found = std::find(m_vListenersRef.cbegin(), m_vListenersRef.cend(), pcListener); found != m_vListenersRef.cend()) {
+		return;
 	}
+
 	//追加
 	m_vListenersRef.push_back(pcListener);
 }
 
-void CSubject::_RemoveListener(CListener* pcListener)
+/*!
+ * リスナーを削除する
+ */
+void CSubject::_RemoveListener(CListener* pcListener) noexcept
 {
+	//NULLは追加できないので、削除も想定しない
+	assert(pcListener);
+
+	//削除前のリスナー数を記録
+	const auto llisteners = m_vListenersRef.size();
+
 	//配列から削除
-	for(int i=0;i<(int)m_vListenersRef.size();i++){
-		if(m_vListenersRef[i]==pcListener){
-			m_vListenersRef.erase(m_vListenersRef.begin()+i);
-			break;
+	if (auto found = std::find(m_vListenersRef.begin(), m_vListenersRef.end(), pcListener); found != m_vListenersRef.end()) {
+		//確実に解除するため、リスナー側の終了メソッドを呼び出す
+		pcListener->_EndListen();
+
+		//リスナー数が代わってないときだけ削除に進む
+		if (llisteners == m_vListenersRef.size()) {
+			//リスナー配列から削除
+			m_vListenersRef.erase(found);
 		}
 	}
 }
@@ -51,31 +66,48 @@ void CSubject::_RemoveListener(CListener* pcListener)
 //                         CListener                           //
 // -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- //
 
-CListener::CListener()
-: m_pcSubjectRef(nullptr)
-{
-}
+CListener::CListener() = default;
 
 CListener::~CListener()
 {
-	Listen(nullptr);
+	_EndListen();
 }
 
-CSubject* CListener::Listen(CSubject* pcSubject)
+/*!
+ * 監視を開始する
+ */
+CSubject* CListener::Listen(
+	CSubject* pcSubject	//!< [in] 監視するサブジェクト。NULLで監視終了
+)
 {
-	CSubject* pOld = GetListeningSubject();
+	//操作前の参照を取得
+	auto pOld = GetListeningSubject();
 
-	//古いサブジェクトを解除
-	if(m_pcSubjectRef){
-		m_pcSubjectRef->_RemoveListener(this);
-		m_pcSubjectRef = nullptr;
-	}
+	//古いサブジェクトを解放
+	_EndListen();
 
 	//新しく設定
-	m_pcSubjectRef = pcSubject;
-	if(m_pcSubjectRef){
-		m_pcSubjectRef->_AddListener(this);
+	if (pcSubject) {
+		//サブジェクト側にリスナー追加を要求する
+		pcSubject->_AddListener(this);
+
+		//参照を保存
+		m_pcSubjectRef = pcSubject;
 	}
 
 	return pOld;
 }
+
+/*!
+ * 監視を終了する
+ */
+void CListener::_EndListen() noexcept
+{
+	if (auto pcSubject = m_pcSubjectRef) {
+		//構造的に再帰呼出なので、先に参照を消しておく
+		m_pcSubjectRef = nullptr;
+
+		//サブジェクト側にリスナー削除を要求する
+		pcSubject->_RemoveListener(this);
+	}
+}

sakura_core/util/relation_tool.h

@@ -36,7 +36,7 @@ class CSubject{
 public:
 	//管理用
 	void _AddListener(CListener* pcListener);
-	void _RemoveListener(CListener* pcListener);
+	void _RemoveListener(CListener* pcListener) noexcept;
 
 private:
 	std::vector<CListener*> m_vListenersRef;
@@ -58,8 +58,11 @@ class CListener{
 	CSubject* Listen(CSubject* pcSubject); //!< 直前にウォッチしていたサブジェクトを返す
 	CSubject* GetListeningSubject() const{ return m_pcSubjectRef; }
 
+	//管理用
+	void		_EndListen() noexcept;
+
 private:
-	CSubject* m_pcSubjectRef;
+	CSubject* m_pcSubjectRef = nullptr;
 };
 
 template <class LISTENER> class CSubjectT : public CSubject{

tests/unittests/test-cppa.cpp

@@ -8,6 +8,13 @@
 
 #include "macro/CPPA.h"
 
+#include "doc/CEditDoc.h"
+#include "env/CShareData.h"
+#include "macro/CSMacroMgr.h"
+#include "view/CEditView.h"
+
+#include "eval_outputs.hpp"
+
 /*!
 	CPPA::GetDllNameImpのテスト
  */
@@ -76,3 +83,73 @@ TEST(CPPA, GetDeclarations)
 	cPpa.GetDeclarations(funcInfo8, buffer.data());
 	EXPECT_STREQ("function S_Func4(i0: Integer; s1: string; i2: Integer; s3: string; i4: Integer; s5: string; i6: Integer; s7: string): string; index 8;", buffer.data());
 }
+
+/*!
+ * CPPAエラー情報コールバックのテスト
+ *
+ * 実装が想定するメッセージを出力できるかチェックする
+ * 本来は確認ケースを分割すべきだが、初期化に手間がかかるため1つにまとめている
+ */
+TEST(CPPA, ppaErrorProc)
+{
+	// 共有メモリのインスタンスを生成する
+	const auto pcShareData = std::make_unique<CShareData>();
+
+	// 共有メモリを初期化する
+	pcShareData->InitShareData();
+
+	// ドキュメントの初期化前に文字幅キャッシュの生成が必要
+	SelectCharWidthCache(CWM_FONT_EDIT, CWM_CACHE_SHARE);
+	InitCharWidthCache(GetDllShareData().m_Common.m_sView.m_lf);
+
+	// CEditViewをインスタンス化するにはドキュメントのインスタンスが必要
+	const auto pcEditDoc = std::make_unique<CEditDoc>(nullptr);
+
+	// CEditViewを用意する
+	const auto pcEditView = std::make_unique<CEditView>();
+
+	// SMacroMgrを用意する
+	const auto pcSMacroMgr = std::make_unique<CSMacroMgr>();
+
+	// PPA実行情報を用意する
+	CPPA::PpaExecInfo info{};
+	info.m_pShareData = &GetDllShareData();
+	info.m_pcEditView = pcEditView.get();
+
+	// 既にエラーフラグが立っていたらメッセージは出さない
+	info.m_bError = true;
+	CPPA::CallErrorProc(info, int(F_FILENEW) + 1, nullptr);
+
+	// コマンドエラー
+	info.m_bError = false;
+	EXPECT_ERROUT(CPPA::CallErrorProc(info, int(F_FILENEW) + 1, nullptr), L"関数の実行エラー\nprocedure S_FileNew; index 30101;");
+
+	// 関数エラー
+	info.m_bError = false;
+	EXPECT_ERROUT(CPPA::CallErrorProc(info, int(F_GETFILENAME) + 1, nullptr), L"関数の実行エラー\nfunction S_GetFilename: string; index 40001;");
+
+	// 不明な関数エラー
+	info.m_bError = false;
+	EXPECT_ERROUT(CPPA::CallErrorProc(info, 1 + 1, nullptr), L"不明な関数の実行エラー(バグです)\nFunc_ID=1");
+
+	// エラー情報が不正
+	info.m_bError = false;
+	EXPECT_ERROUT(CPPA::CallErrorProc(info, 0, nullptr), L"エラー情報が不正");
+
+	// 詳細不明のPPAエラー
+	info.m_bError = false;
+	EXPECT_ERROUT(CPPA::CallErrorProc(info, 0, ""), L"詳細不明のエラー");
+
+	// 詳細ありのPPAエラー
+	info.m_bError = false;
+	EXPECT_ERROUT(CPPA::CallErrorProc(info, 0, "test"), L"test");
+
+	// 未定義のエラー
+	info.m_bError = false;
+	EXPECT_ERROUT(CPPA::CallErrorProc(info, -1, "test"), L"未定義のエラー\nError_CD=-1\ntest");
+
+	// デバッグ情報付きのエラー
+	info.m_cMemDebug = "debug";
+	info.m_bError = false;
+	EXPECT_ERROUT(CPPA::CallErrorProc(info, 0, nullptr), L"エラー情報が不正\ndebug");
+}