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

_wcsdupでdeleteしてる #1236

Closed
usagisita opened this issue Apr 22, 2020 · 9 comments
Closed

_wcsdupでdeleteしてる #1236

usagisita opened this issue Apr 22, 2020 · 9 comments
Labels
🐛bug🦋 ■バグ修正(Something isn't working)

Comments

@usagisita
Copy link
Contributor

問題内容

_wcsdup()ではfree()を使うべきはずなのに、メモリー削除にdelete [] を使用している。
実害等あるかは分かりませんが、正常な状態とは思いません。
欲を言うなら、可能な場所(Command_MENU_RBUTTON)では、_wcsdup()よりもstd::wstring/unique_ptr<WCHAR>などを受け皿にして、手動でメモリーを管理するのをやめたほうがいいと思います。


delete [] m_OlmArr[i].pszName;

delete [] m_SIndentArr[i].pszName;

delete []pszFile;

delete[] szUrlDup;

再現手順

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

再現頻度

100%

問題のカテゴリ

  • その他の問題

環境情報

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

ds14050 commented Apr 23, 2020

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

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

@berryzplus
Copy link
Contributor

これは宗教ですぜ?w

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

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

@beru
Copy link
Contributor

beru commented Apr 24, 2020

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

@berryzplus
Copy link
Contributor

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

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

@m-tmatma
Copy link
Member

これは宗教ですぜ?w

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

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

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

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

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

@k-takata
Copy link
Member

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

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

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

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

@berryzplus
Copy link
Contributor

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

@berryzplus
Copy link
Contributor

#1236 (comment) はそのままにしときます。

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

@berryzplus
Copy link
Contributor

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

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

@beru beru added the 🐛bug🦋 ■バグ修正(Something isn't working) label Mar 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛bug🦋 ■バグ修正(Something isn't working)
Projects
None yet
Development

No branches or pull requests

6 participants