参考出品。 new/deleteで自前管理しているバッファのうちある程度を、vectorやauto_ptrなどに置き換えます。 出力exeのファイルサイズが大きくなった気がします。
こんにちは。 私には決定権ないけどレビュってみようかと思って 1件見て、「えええっ」となったのでお知らせします。。。
だいぶ端折ります。 --- sakura_core/_main/CCommandLine.cpp (リビジョン 4150) - LPTSTR pszCmdLineWork = new TCHAR[lstrlen( pszCmdLineSrc ) + 1]; + std::vector<TCHAR> vecCmdLineWork(lstrlen( pszCmdLineSrc ) + 1); + LPTSTR pszCmdLineWork = &vecCmdLineWork[0];
ベクタはベクタで、STLの高機能配列クラスです。 配列クラスの機能を一切使う予定がないのに、 わざわざ重たいラッパー使うのは違う気がします。 sakuraはC/C++混在のプロジェクトなので、 本来とは違う用途でSTLコンテナを入れるのは危険だと思います。
プログラムサイズについてですが、 vs2005のリリースビルドと比べて3kほど増量していました。 サイズ増減はブロック単位なので2、3段階重たくなった感じです。
わたしなら + size_t tmpBuffLen = ::_tcslen(pszCmdLenSrc) + 1; + std:auto_ptr<TCHAR> tmpBuff(new TCHAR[tmpBuffLen]); + std::fill_n(&tmpBuff, tmpBuffLen, TCHAR()); + LPTSTR pszCmdLineWork = &tmpBuff; とか + std:basic_string<TCHAR> tstrBuff(::_tcslen(pszCmdLineSrc), TCHAR()); + LPTSTR pszCmdLineWork = &*tstrBuff.begin(); とします。
auto_ptrみたいな、薄いラッパーにメモリ管理だけ任せる(初期化は自前)とか、 同じ高機能ラッパでも「文字列」クラスを使うとか、 使いまわしが効くようにshared_ptrを使うか(c++11だから無理ｗ)。
lstrlenの話はまたにします。
以上です。
訂正。 増量サイズは30K
before: 1.50 MB (1,576,960 バイト) after: 1.53 MB (1,605,632 バイト)
不必要にベクタ使ってるところ削ったら ファイル小さくなるんじゃないか？ ってくらい大きな違いが出てました。 （個人的に「配列」がキラいなだけなので鵜呑みにしないでくださいｗ）
ご意見ありがとうございます。 auto_ptrはC++11以降、非推奨なので本当はあまり増やしたくないです。 あと現状でauto_ptrを使うと、new/deleteが対照的にコードに現れなくていまいちな気がします。 さらに、こっちの方が問題なんですが、auto_ptrは配列を管理させることはもともとできません。 auto_array_ptrが代わりにsakuraでは用意されているものの、newが必要なのは相変わらずです。
vectorも配列長が0のときに &data[0]とかで参照すると例外になってしまうので、本来ならあまりいい方法ではないです。 確かにTCHAR/wchar_tなら、basic::stringのほうがいいかもしれません。そのへんは参考程度なので細かく考えずに適当に書いた名残です。 例に上げたCCommandLine.cppなどでは、strtokが使われていて、バッファを書き換える必要があります。 連続性が保証されていて、書き換え可能である必要があるので、basic::stringはC++03では不適当のような「気がしました」。 特にサクラエディタでは、古いg++/STLを採用することも可能なはずなので、std::stringがcopy-on-writeだったはずで、ちょっと困る「かもしれない」です。
サイズが増えている原因を特定するのは難しいですが、テンプレート展開された未使用関数は最適化で削除されるといいなという淡い期待があります。 実際に増えてる所を見るとそうでもないんでしょうか。 関数で例外発生時にメモリーを解放させるコードなどが増えているはずで、それはvectorでなく他を採用しようとも増えると思うんですよね。
strlen について先に書いておくと、 サクラエディタには auto_srtlen というのがあり、変数の型を書き換えてもchar/wchar_tなら自動追随して処理されるので、個人的にはお勧めです。 なぜかパッチでもそれを採用してなかったりしますけど。
とりあえずこのパッチは参考品で、個人的には(私が決める訳でもないけど)採用予定はないので、お気楽にどうぞ。 重要なところで自信がないのは私が日曜プログラマだからなので、気をつけてください。
こんばんは。C#を始めて2か月目の初心者です♪ WindowsAPIの混じったコードを読み書きできる人にプロもアマもないような。 「達人への道は転がり落ちるものだよ」と何かの漫画に書いてありました。
auto_ptrが配列ポインタに使えない、というのは誤解です。 その話を真に受けると::GlobalAllocで確保したメモリを::GlobalFreeで開放するのはキケンて話になります。 ・・・そんなバカな話はありません。
auto_ptrもメモリ開放まではやってくれるんです。 やってくれないのは個々のメンバのデストラクタ呼出しだけです。 もしそこに重要な開放処理があるクラスを突っ込んだとすると、 リソースリークが発生して大変ヤヴァいことが起きてしまいます。 だから言語仕様的に「使わないほうがよくね？」となったわけです。 charとかhwndとか入れる分には問題ありません。 HBRUSHとかHGLOBALを入れるのはヤヴァいかも知れません。
basic_stringですが、テンプレートクラスなのでuint32_tとかでも動きます。 メモリ書換えして途中にNULが入ると変な動きをする懸念はありますが、 肝を押さえておけば簡易バッファとして便利です。
std::string nullStr(MAX_PATH, NULL); //261バイト分0埋めのchar配列が出来る std::out << nullStr << std::endl; //空行が出力される std::out << &nullStr.begin() << std::endl; //同じ。length=260なのでエラーにならない nullStr[0] = 'a';//これもエラーにならない。ただしlengthは更新されず260のまま。 nullStr = &nullStr.begin(); //内容が"a"になる。length=1になる。 ※変数名nullStrにNULじゃないものが入るのはどうなんだ、というツッコミは置いといて。
sakuraのauto_系関数は、個人的には非推奨です。 理由はいくつかありますが、CRT関数を使える場所には標準のCRT関数を使うべきだからです。
VC++のCRT関数群は、状況によりインライン化される場合があります。 この場合のインライン化は「速くなる」or「サイズが小さくなる」です。 極端な例でいくと::_tcslen("abc") → 3 のようにインライン化されます。
lstrlenWのようにKernel32のエクスポート関数を直接指定してしまうと、 せっかくある高度なコンパイラ最適化機能の恩恵を受けられません。 そもそもlstrlenのような似非CRT関数は、OSが最低限提供すべき互換性を保つために提供されているものです。特殊な場合を除いて使うべきものじゃないです。
その話よりも先に、初期化やエラー処理が抜けてるのが気になるわけです。。。
こんばんは。 ちょっと気になってauto_array_ptrのソースを見てみました。 絶対に使うべきでないクラスです。
v2のベースがkabake氏の独自拡張ブランチだと知りつつ、 ヤヴァいものはヤヴァいと言っておきます。
「配列対応」を謳っていて「デタッチ」機能があるように見えますが、 デストラクタ内の「delete[] m_array;」はノーチェックで呼ばれています。 デタッチしたあとにこいつを破棄すると、普通に例外で落ちます。 問題になってないのは「たまたま」でしょう。
使えないことはありませんが、積極的に使うべきものじゃないです。 現状でヤヴァいのは１か所だけで、理由はauto_array_ptrの本質的なヤヴァさと無関係です。 他は問題ないので、心配するようなこっちゃないです。
svn\sakura2\sakura_core\macro\CWSHIfObj.cpp(121) : auto_array_ptr<VARIANTARG> rgvargParam( new VARIANTARG[ArgCount] ); 128行目と145行目の、エラーリターンの際に、コピー済みのVariantがリークします。 （気付いてなかったけど他にもあるのかなぁ…）
これなんかどうでしょう。 VariantとVARIANTをごまかしていったり来たりさせてるのが元凶だと思うんですが。 \sakura_core\macro\CMacro.cpp (1267): VARIANT vArg[2]; // HandleFunctionに渡す引数 VARIANT vResult; // HandleFunctionから返る値 (省略) return HandleFunction( pcEditView, Index, vArg, nArgSize, vResult ); たぶん戻ってくるvResultとvArg[1]は整数型だから「動作上だけ見れば」セーフ？ vArg[0]の中身文字列みたいな気がする。
漏れてますねぇ…。ご指摘の通りです。
仮にvArg[0]がintとか開放不要な型だったとしても 呼ばれた先で型変換してBSTRに変えてるのでそいつが漏れます。
っていうか、このルートを通る呼出って常に「true」が返りますけどいいんでしたっけ？ 「はい／いいえ」を選択させるメッセージを出す場合は、 どちらが押されたか知りたいと思うのが人情のような。
[C++] 配列の使い分け？について https://teratail.com/questions/68935 new [] vs vectorの話です。 流し読みしていたら見つけたので参考までにどうぞ。
こんにちは。URL見ました。 議論されている内容に、とくに違和感はありません。
一応補足すると、 Hogeなんて独自クラス作らずにstd::vector<int>使いましょう、 ってのが私の意見です。
typedef std::vector<int> Hoge; ということ。
int をメンバに持つ独自クラスを作る話と ローカル変数int をstd::vector<int>に書き換える話では、 考えるポイントが違ってくると思います。
「int* をメンバに持つ独自クラス」にあたるものが、 既にたくさん存在していることは別のお話。
気になっていた誤りについて自己レス。
デストラクタ内の「delete[] m_array;」はノーチェックで呼ばれています。
delete/delete[]は「引数がnullptrの場合、何もしない」が言語仕様なのでノーチェックで問題ありません。
Log in to post a comment.
こんにちは。
私には決定権ないけどレビュってみようかと思って
1件見て、「えええっ」となったのでお知らせします。。。
だいぶ端折ります。
--- sakura_core/_main/CCommandLine.cpp (リビジョン 4150)
- LPTSTR pszCmdLineWork = new TCHAR[lstrlen( pszCmdLineSrc ) + 1];
+ std::vector<TCHAR> vecCmdLineWork(lstrlen( pszCmdLineSrc ) + 1);
+ LPTSTR pszCmdLineWork = &vecCmdLineWork[0];
ベクタはベクタで、STLの高機能配列クラスです。
配列クラスの機能を一切使う予定がないのに、
わざわざ重たいラッパー使うのは違う気がします。
sakuraはC/C++混在のプロジェクトなので、
本来とは違う用途でSTLコンテナを入れるのは危険だと思います。
プログラムサイズについてですが、
vs2005のリリースビルドと比べて3kほど増量していました。
サイズ増減はブロック単位なので2、3段階重たくなった感じです。
わたしなら
+ size_t tmpBuffLen = ::_tcslen(pszCmdLenSrc) + 1;
+ std:auto_ptr<TCHAR> tmpBuff(new TCHAR[tmpBuffLen]);
+ std::fill_n(&tmpBuff, tmpBuffLen, TCHAR());
+ LPTSTR pszCmdLineWork = &tmpBuff;
とか
+ std:basic_string<TCHAR> tstrBuff(::_tcslen(pszCmdLineSrc), TCHAR());
+ LPTSTR pszCmdLineWork = &*tstrBuff.begin();
とします。
auto_ptrみたいな、薄いラッパーにメモリ管理だけ任せる(初期化は自前)とか、
同じ高機能ラッパでも「文字列」クラスを使うとか、
使いまわしが効くようにshared_ptrを使うか(c++11だから無理ｗ)。
lstrlenの話はまたにします。
以上です。
訂正。
増量サイズは30K
before: 1.50 MB (1,576,960 バイト)
after: 1.53 MB (1,605,632 バイト)
不必要にベクタ使ってるところ削ったら
ファイル小さくなるんじゃないか？
ってくらい大きな違いが出てました。
（個人的に「配列」がキラいなだけなので鵜呑みにしないでくださいｗ）
以上です。
ご意見ありがとうございます。
auto_ptrはC++11以降、非推奨なので本当はあまり増やしたくないです。
あと現状でauto_ptrを使うと、new/deleteが対照的にコードに現れなくていまいちな気がします。
さらに、こっちの方が問題なんですが、auto_ptrは配列を管理させることはもともとできません。
auto_array_ptrが代わりにsakuraでは用意されているものの、newが必要なのは相変わらずです。
vectorも配列長が0のときに &data[0]とかで参照すると例外になってしまうので、本来ならあまりいい方法ではないです。
確かにTCHAR/wchar_tなら、basic::stringのほうがいいかもしれません。そのへんは参考程度なので細かく考えずに適当に書いた名残です。
例に上げたCCommandLine.cppなどでは、strtokが使われていて、バッファを書き換える必要があります。
連続性が保証されていて、書き換え可能である必要があるので、basic::stringはC++03では不適当のような「気がしました」。
特にサクラエディタでは、古いg++/STLを採用することも可能なはずなので、std::stringがcopy-on-writeだったはずで、ちょっと困る「かもしれない」です。
サイズが増えている原因を特定するのは難しいですが、テンプレート展開された未使用関数は最適化で削除されるといいなという淡い期待があります。
実際に増えてる所を見るとそうでもないんでしょうか。
関数で例外発生時にメモリーを解放させるコードなどが増えているはずで、それはvectorでなく他を採用しようとも増えると思うんですよね。
strlen について先に書いておくと、
サクラエディタには auto_srtlen というのがあり、変数の型を書き換えてもchar/wchar_tなら自動追随して処理されるので、個人的にはお勧めです。
なぜかパッチでもそれを採用してなかったりしますけど。
とりあえずこのパッチは参考品で、個人的には(私が決める訳でもないけど)採用予定はないので、お気楽にどうぞ。
重要なところで自信がないのは私が日曜プログラマだからなので、気をつけてください。
こんばんは。C#を始めて2か月目の初心者です♪
WindowsAPIの混じったコードを読み書きできる人にプロもアマもないような。
「達人への道は転がり落ちるものだよ」と何かの漫画に書いてありました。
auto_ptrが配列ポインタに使えない、というのは誤解です。
その話を真に受けると::GlobalAllocで確保したメモリを::GlobalFreeで開放するのはキケンて話になります。
・・・そんなバカな話はありません。
auto_ptrもメモリ開放まではやってくれるんです。
やってくれないのは個々のメンバのデストラクタ呼出しだけです。
もしそこに重要な開放処理があるクラスを突っ込んだとすると、
リソースリークが発生して大変ヤヴァいことが起きてしまいます。
だから言語仕様的に「使わないほうがよくね？」となったわけです。
charとかhwndとか入れる分には問題ありません。
HBRUSHとかHGLOBALを入れるのはヤヴァいかも知れません。
basic_stringですが、テンプレートクラスなのでuint32_tとかでも動きます。
メモリ書換えして途中にNULが入ると変な動きをする懸念はありますが、
肝を押さえておけば簡易バッファとして便利です。
std::string nullStr(MAX_PATH, NULL); //261バイト分0埋めのchar配列が出来る
std::out << nullStr << std::endl; //空行が出力される
std::out << &nullStr.begin() << std::endl; //同じ。length=260なのでエラーにならない
nullStr[0] = 'a';//これもエラーにならない。ただしlengthは更新されず260のまま。
nullStr = &nullStr.begin(); //内容が"a"になる。length=1になる。
※変数名nullStrにNULじゃないものが入るのはどうなんだ、というツッコミは置いといて。
sakuraのauto_系関数は、個人的には非推奨です。
理由はいくつかありますが、CRT関数を使える場所には標準のCRT関数を使うべきだからです。
VC++のCRT関数群は、状況によりインライン化される場合があります。
この場合のインライン化は「速くなる」or「サイズが小さくなる」です。
極端な例でいくと::_tcslen("abc") → 3 のようにインライン化されます。
lstrlenWのようにKernel32のエクスポート関数を直接指定してしまうと、
せっかくある高度なコンパイラ最適化機能の恩恵を受けられません。
そもそもlstrlenのような似非CRT関数は、OSが最低限提供すべき互換性を保つために提供されているものです。特殊な場合を除いて使うべきものじゃないです。
その話よりも先に、初期化やエラー処理が抜けてるのが気になるわけです。。。
こんばんは。
ちょっと気になってauto_array_ptrのソースを見てみました。
絶対に使うべきでないクラスです。
v2のベースがkabake氏の独自拡張ブランチだと知りつつ、
ヤヴァいものはヤヴァいと言っておきます。
「配列対応」を謳っていて「デタッチ」機能があるように見えますが、
デストラクタ内の「delete[] m_array;」はノーチェックで呼ばれています。
デタッチしたあとにこいつを破棄すると、普通に例外で落ちます。
問題になってないのは「たまたま」でしょう。
使えないことはありませんが、積極的に使うべきものじゃないです。
現状でヤヴァいのは１か所だけで、理由はauto_array_ptrの本質的なヤヴァさと無関係です。
他は問題ないので、心配するようなこっちゃないです。
svn\sakura2\sakura_core\macro\CWSHIfObj.cpp(121)
: auto_array_ptr<VARIANTARG> rgvargParam( new VARIANTARG[ArgCount] );
128行目と145行目の、エラーリターンの際に、コピー済みのVariantがリークします。
（気付いてなかったけど他にもあるのかなぁ…）
これなんかどうでしょう。
VariantとVARIANTをごまかしていったり来たりさせてるのが元凶だと思うんですが。
\sakura_core\macro\CMacro.cpp (1267):
VARIANT vArg[2]; // HandleFunctionに渡す引数
VARIANT vResult; // HandleFunctionから返る値
(省略)
return HandleFunction( pcEditView, Index, vArg, nArgSize, vResult );
たぶん戻ってくるvResultとvArg[1]は整数型だから「動作上だけ見れば」セーフ？
vArg[0]の中身文字列みたいな気がする。
漏れてますねぇ…。ご指摘の通りです。
仮にvArg[0]がintとか開放不要な型だったとしても
呼ばれた先で型変換してBSTRに変えてるのでそいつが漏れます。
っていうか、このルートを通る呼出って常に「true」が返りますけどいいんでしたっけ？
「はい／いいえ」を選択させるメッセージを出す場合は、
どちらが押されたか知りたいと思うのが人情のような。
[C++] 配列の使い分け？について
https://teratail.com/questions/68935
new [] vs vectorの話です。
流し読みしていたら見つけたので参考までにどうぞ。
こんにちは。URL見ました。
議論されている内容に、とくに違和感はありません。
一応補足すると、
Hogeなんて独自クラス作らずにstd::vector<int>使いましょう、
ってのが私の意見です。
typedef std::vector<int> Hoge;
ということ。
int をメンバに持つ独自クラスを作る話と
ローカル変数int をstd::vector<int>に書き換える話では、
考えるポイントが違ってくると思います。
「int* をメンバに持つ独自クラス」にあたるものが、
既にたくさん存在していることは別のお話。
Last edit: berryzplus 2017-03-18
気になっていた誤りについて自己レス。
delete/delete[]は「引数がnullptrの場合、何もしない」が言語仕様なのでノーチェックで問題ありません。