Sakura: _wcsdupでdeleteしてる

Created on 22 Apr 2020  ·  9Comments  ·  Source: sakura-editor/sakura

問題内容

_wcsdup()ではfree()を使うべきはずなのに、メモリー削除にdelete [] を使用している。
実害等あるかは分かりませんが、正常な状態とは思いません。
欲を言うなら、可能な場所(Command_MENU_RBUTTON)では、_wcsdup()よりもstd::wstring/unique_ptr\

https://github.com/sakura-editor/sakura/blob/948ce7e2ea0c230270c87a092860cda6756095c3/sakura_core/cmd/CViewCommander_CustMenu.cpp#L52
https://github.com/sakura-editor/sakura/blob/948ce7e2ea0c230270c87a092860cda6756095c3/sakura_core/typeprop/CPropTypesScreen.cpp#L804
https://github.com/sakura-editor/sakura/blob/948ce7e2ea0c230270c87a092860cda6756095c3/sakura_core/typeprop/CPropTypesScreen.cpp#L829
https://github.com/sakura-editor/sakura/blob/948ce7e2ea0c230270c87a092860cda6756095c3/sakura_core/view/CEditView_Mouse.cpp#L1534
https://github.com/sakura-editor/sakura/blob/948ce7e2ea0c230270c87a092860cda6756095c3/sakura_core/view/CEditView_Mouse.cpp#L1593

再現手順

(ソースコード閲覧)
該当部分を実行してメモリーリーク検出ツールなどを使うと、何か起こる可能性はあります。

再現頻度

100%

問題のカテゴリ

  • その他の問題

環境情報

  • OS バージョン
    無関係(ソースコード上の問題)
  • サクラエディタバージョン
    2.4.0.0

All 9 comments

基本的に根っこはこの2つでしょうか。

ご本人に降臨頂いて繰り返さないように学習してもらいたいところです。

これは宗教ですぜ?w

MSVCの実装だとnew/deleteとmalloc/freeは同じなので気にしなくていいと思います。

とはいえ、確保と解放のペアが不一致なのはキモいので対応することは否定しない感じです。

お、保守が面倒なmingwビルドを捨てる事を許容するかのごとくなコメントが!

いや、破棄する意図はないです。

mingwのcrtはmsvcrtを流用してるのでGucci
のコダワリ仕様を除いてmsvc互換だと思っています。

これは宗教ですぜ?w

MSVCの実装だとnew/deleteとmalloc/freeは同じなので気にしなくていいと思います。

とはいえ、確保と解放のペアが不一致なのはキモいので対応することは否定しない感じです。

実装に依存して実装するのか、インターフェース仕様に基づいて実装するかだと思います。

実装に依存していると、違うコンパイラ使ったり、内部実装が変わった時に動かなくなったり、意図しない動作をしたりするのでよろしくないです。

私がよく、関数にコメント書いて、というのは、関数の仕様を明確にしたいからです。
関数の仕様が明確だと、仕様を変えない範囲で自由に関数内部の実装を変えられるということもあります。

MSVCの実装だとnew/deleteとmalloc/freeは同じなので気にしなくていいと思います。

https://docs.microsoft.com/ja-jp/cpp/cpp/delete-operator-cpp?view=vs-2019

Newで割り当てられていないオブジェクトへのポインターに対してdeleteを使用すると、予測できない結果になります。

今のバージョンではたまたま動いているだけであって、鼻から悪魔が出ても文句は言えません。

iPhoneの単語補完/単語補正の機能は、たまにマジ困る・・・。
前のコメント、iphoneで急いで打ったんですが、
GCCGucci に変えてくれてたのに気付きませんでした。

https://github.com/sakura-editor/sakura/issues/1236#issuecomment-618782393 はそのままにしときます。

オイラがアフォでも、適切な指摘で訂正してくれる人がいれば、全体として問題はないわけで、安心してアフォで居られるということなので。

よく見たら #1239 がマージされて、ここで指摘された _wcsdup() で確保したメモリを delete[] で解放しているコードはなくなったので、issueは閉じておきます。

結論は、いまは問題なくても仕様的にマズい(根拠あり)で、適切に対応(delete[]ではなくfree()を使うように修正)することにして対処しました、です。

Was this page helpful?
0 / 5 - 0 ratings