構造体を動的に確保＆解放

質問

度々お世話になってます。

構造体を動的に確保し、処理後メモリーリークが起こらないように解放したいです。
item を m_xcList.GetItemCount() 個、宣言したいです。

ご教授願いたいです。

▼ソースの一部抜粋▼

void CFileListCreatorDlg::SameItemCheck(CString mySwitch)
{

　LVITEM　 lvi;
　int　　 index = 0;

　UpdateData();
　lvi.mask = LVIF_TEXT;

　CString FullPathString;
　CString myFileName;

　index = 0;
　while (index < CFileListCreatorDlg::m_xcList.GetItemCount()){
　　lvi.iItem = index;
　　lvi.iSubItem = 0;　　　

　　lvi.pszText = const_cast<LPTSTR>(static_cast<LPCTSTR>(_T("")));
　　CFileListCreatorDlg::m_xcList.SetItem(&lvi);
　　index++;
　}

　//0:ファイル重複識別ナンバー 1:通し番号 2:フルパス 3:ファイル名 4:おおよそのデータサイズ 5:データサイズ 6:修正日 7:修正時間 8:備考欄 9:書式情報
　//ファイル名、ファイル容量、修正日時により、重複チェック

　struct item
　{
　　CString　RepetitionNum;　//ファイル重複識別ナンバー

　　//int　　Num;　　//通し番号

　　CString　FullPath;　//ファイルパス
　　CString　FileName;　//ファイル名

　　//CString　ApproximateByte;　//おおよそのデータサイズ//バイト

　　CString　AccurateByte;　　//実際のデータサイズ//実バイト

　　//TCHAR　ModifyDate[20];　　//修正日
　　//CTime　ModifyTime;　　//修正時間

　　CString　ModifyTime;　　//修正時間

　　//TCHAR　RemarksColumn[100];　　//備考欄
　};

　//http://www-watt.mech.eng.osaka-u.ac.jp/~tasai/cp/lec1.html　← プログラム５を参考にしました。

　struct item *array;

　/* メモリの確保 */
　array = (struct item*)malloc(sizeof(struct item)*(CFileListCreatorDlg::m_xcList.GetItemCount()-1));
　if (array == NULL) {
　　StatusStringSet(_T("memory allocation error\r\n"),0,TRUE);
　　exit(EXIT_FAILURE);
　}

　//const int itemNum = sizeof item /sizeof array[0];

　int i;

　for (i = 0; i < CFileListCreatorDlg::m_xcList.GetItemCount(); i++) //初期化
　{
　　array[i].RepetitionNum = _T(""); //ファイル重複識別ナンバー
　　array[i].FileName = _T(""); //ファイル名
　　array[i].AccurateByte = _T(""); //データサイズ
　　array[i].ModifyTime = _T(""); //修正時間
　}

int cc;
　int dd;

　cc = 1;
　dd = 1;

　if(mySwitch == _T("SameItemCheck_BY_FileName")){
　　//0:ファイル重複識別ナンバー 1:通し番号 2:フルパス 3:ファイル名 4:おおよそのデータサイズ 5:データサイズ 6:修正日 7:修正時間 8:備考欄 9:書式情報
　　StatusStringSet(_T("ファイル名により、重複チェック"),0,FALSE);
　　index = 0;

　　while (index < CFileListCreatorDlg::m_xcList.GetItemCount()){
　　　array[index].RepetitionNum = _T("");

　　　FullPathString = CFileListCreatorDlg::m_xcList.GetItemText(index,2);

　　　if(FullPathString != _T("")){
　　　　array[index].FileName = FullPathToFileName(FullPathString);
　　　}

　　　for (dd = 0; dd <= index-1; dd++){
　　　　if (array[dd].FileName == array[index].FileName){
　　　　　if (array[index].RepetitionNum != _T("")){
　　　　　　array[dd].RepetitionNum = array[index].RepetitionNum;
　　　　　　//" 重複番号欄に既に重複Ｎｏが、入っているとき"
　　　　　}else{
　　　　　　//cc // " インクリメントcc"
　　　　　　　CString str;
　　　　　　　if (cc>=INT_MAX){
　　　　　　　　str = _T("MAX-Value");
　　　　　　　}else{
　　　　　　　　str.Format(_T("%d"),cc);
　　　　　　　}

　　　　　　　array[index].RepetitionNum = const_cast<LPTSTR>(static_cast<LPCTSTR>(str));
　　　　　　　if (array[dd].RepetitionNum != array[index].RepetitionNum){
　　　　　　　　cc++;
　　　　　　　}

　　　　　}

　　　　　lvi.iItem = index;
　　　　　lvi.iSubItem = 0;　　　

　　　　　lvi.pszText = const_cast<LPTSTR>(static_cast<LPCTSTR>(array[index].RepetitionNum));
　　　　　CFileListCreatorDlg::m_xcList.SetItem(&lvi);
　　　　　UpdateData(FALSE);

　　　　　lvi.iItem = dd;
　　　　　lvi.iSubItem = 0;　　　

　　　　　lvi.pszText = const_cast<LPTSTR>(static_cast<LPCTSTR>(array[index].RepetitionNum));
　　　　　CFileListCreatorDlg::m_xcList.SetItem(&lvi);
　　　　　UpdateData(FALSE);
　　　　}
　　　}
　　　index++;

　　　UpdateData(FALSE);
　　}
　}else if(mySwitch == _T("SameItemCheck_By_DataSize")){
　　//省略
　}
}
▲ソースの一部抜粋▲

コンパイルは通るのですが、実行時に↓のエラーがでます。
どこの部分ががおかしいのでしょうか？？

　▼エラーメッセージ▼

　FileListCreator.exe の 0x00f08ac1 でハンドルされていない例外が発生しました: 0xC0000005: 場所 0xcdcdcdbd を読み込み中にアクセス違反が発生しました。

　void Empty() throw()
　{
　　CStringData* pOldData = GetData();
　　IAtlStringMgr* pStringMgr = pOldData->pStringMgr;
　　if( pOldData->nDataLength == 0 )
　　{
　　　return;
　　}

　　if( pOldData->IsLocked() )
　　{
　　　// Don't reallocate a locked buffer that's shrinking
　　　SetLength( 0 );
　　}
　　else
　　{
　　　pOldData->Release();
　　　CStringData* pNewData = pStringMgr->GetNilString();
　　　Attach( pNewData );
　　}
　}

　▲エラーメッセージ▲

汎用的なコードをお教え願いたいです。宜しくお願いします。

回答No.4

Tacosan · ベストアンサー

#include <stdio.h>

int main()
{
int a[5];
for (i=1; i<=5; ++i)
a[i] = i*i;
for (i=1; i<=5; ++i)
printf("a[%d]=%d\n", i, a[i]);
return 0;
}

というプログラムは間違っています. どこが間違いかわかりますか?

お礼

肝心の宣言部が抜けていました。お礼欄にて失礼いたします。

　struct itemStruct
　{
　　CString　RepetitionNum;　//ファイル重複識別ナンバー
　　CString　FullPath;　//ファイルパス
　　CString　FileName;　//ファイル名
　　CString　AccurateByte;　　//実際のデータサイズ//実バイト
　　CString　ModifyTime;　　//修正時間
　};

　itemStruct *item;
　item = new itemStruct[CFileListCreatorDlg::m_xcList.GetItemCount()];

それと、一応、処理部の外でもリストコントールの重複ナンバー欄を初期化しています。
冗長かもしれませんが。

　LVITEM　 lvi;
　int　　 index = 0;

　UpdateData();
　lvi.mask = LVIF_TEXT;

　CString FullPathString;

　index = 0;
　while (index < CFileListCreatorDlg::m_xcList.GetItemCount()){
　　lvi.iItem = index;
　　lvi.iSubItem = 0; //重複ナンバーが入るコラム

　　lvi.pszText = const_cast<LPTSTR>(static_cast<LPCTSTR>(_T("")));
　　CFileListCreatorDlg::m_xcList.SetItem(&lvi);
　　index++;
　}

以上です。

補足

遅くなりました。

【誤】i<=5
【正】i<5

new とか、配列の宣言方法に目を囚われて、すみません。添え字の問題でしたか。
今、動作確認をしました。大丈夫そうです。

同様に、自プログラムの初期化部分と処理部分の終了条件を一つ少なくして、表記をそろえました。

【誤】i <= CFileListCreatorDlg::m_xcList.GetItemCount()
【正】i < CFileListCreatorDlg::m_xcList.GetItemCount()

void CFileListCreatorDlg::SameItemCheck(CString mySwitch)
{

　▼初期化▼

　int i;

　for (i = 0; i < CFileListCreatorDlg::m_xcList.GetItemCount(); i++) //初期化
　{
　　item[i].RepetitionNum = _T(""); //ファイル重複識別ナンバー
　　item[i].FileName = _T(""); //ファイル名
　　item[i].AccurateByte = _T(""); //データサイズ
　　item[i].ModifyTime = _T(""); //修正時間
　}
　▲初期化▲

　▼重複ファイルチェック部分▼

cc = 1;

　if(mySwitch == _T("SameItemCheck_BY_FileName")){
//m_xcListのコラムの添え字↓
　　//0:ファイル重複識別ナンバー 1:通し番号 2:フルパス 3:ファイル名 4:おおよそのデータサイズ 5:データサイズ 6:修正日 7:修正時間 8:備考欄 9:書式情報

　　//ファイル名により、重複チェック
　　index = 0;

　　while (index < CFileListCreatorDlg::m_xcList.GetItemCount()){

　　　FullPathString = CFileListCreatorDlg::m_xcList.GetItemText(index,2);

　　　if(FullPathString != _T("")){
　　　　item[index].FileName = FullPathToFileName(FullPathString); //ファイル名を取得するユーザー関数
　　　}

　　　for (dd = 0; dd < index; dd++){
　　　　if (item[dd].FileName == item[index].FileName){
　　　　　if (item[index].RepetitionNum != _T("")){
　　　　　　item[dd].RepetitionNum = item[index].RepetitionNum;
　　　　　　//" 重複番号欄に既に重複Ｎｏが、入っているとき"
　　　　　}else{
　　　　　　//cc // " インクリメントcc"
　　　　　　　CString str;
　　　　　　　if (cc>=INT_MAX){
　　　　　　　　str = _T("MAX-Value");
　　　　　　　}else{
　　　　　　　　str.Format(_T("%d"),cc);
　　　　　　　}

　　　　　　　item[index].RepetitionNum = const_cast<LPTSTR>(static_cast<LPCTSTR>(str));
　　　　　　　if (item[dd].RepetitionNum != item[index].RepetitionNum){
　　　　　　　　cc++;
　　　　　　　}

　　　　　}

　　　　　lvi.iItem = index;
　　　　　lvi.iSubItem = 0;　　　

　　　　　lvi.pszText = const_cast<LPTSTR>(static_cast<LPCTSTR>(item[index].RepetitionNum));
　　　　　CFileListCreatorDlg::m_xcList.SetItem(&lvi);
　　　　　UpdateData(FALSE);

　　　　　lvi.iItem = dd;
　　　　　lvi.iSubItem = 0;　　　

　　　　　lvi.pszText = const_cast<LPTSTR>(static_cast<LPCTSTR>(item[index].RepetitionNum)); // item[dd] ではなく item[index]の重複ナンバーを代入
　　　　　CFileListCreatorDlg::m_xcList.SetItem(&lvi);
　　　　　UpdateData(FALSE);
　　　　}
　　　}
　　　index++;
　　}

　▲重複ファイルチェック部分▲

　delete [] item;

} //SameItemCheck 終端

これでも、最初の目的であるメモリーリークが防げないようでしたら、お教え下さい。

明日あたりに、締め切ります。ありがとうございました。

回答No.3

Tacosan

あとついでですが C でキャストしなくていいというのは 20年以上昔からそうですし, C++ なら std::vector 使えばいいような.

補足

アドバイスありがとうございます。
でも、今回のところは、new/deleteを採用したいです。

今のところ、↓ですが、どこが悪いのか分かりません。。。

{
　struct itemStruct
　{
　　CString　RepetitionNum;　//ファイル重複識別ナンバー

　　CString　FullPath;　//ファイルパス
　　CString　FileName;　//ファイル名
　　CString　AccurateByte;　　//実際のデータサイズ//実バイト
　　CString　ModifyTime;　　//修正時間
　//} item[] =
　//　{_T(""),_T(""),_T(""),_T(""),_T(""),
　};

　itemStruct *item;
　item = new itemStruct[CFileListCreatorDlg::m_xcList.GetItemCount()];

　int i;

　for (i = 1; i <= CFileListCreatorDlg::m_xcList.GetItemCount(); i++) //初期化
　{
　　item[i].RepetitionNum = _T(""); //ファイル重複識別ナンバー
　　item[i].FileName = _T(""); //ファイル名
　　item[i].AccurateByte = _T(""); //データサイズ
　　item[i].ModifyTime = _T(""); //修正時間
　}

　～処理～

　delete [] item;

}

以上、プログラム抜粋は、実行時エラーで、

　void Empty() throw()
　{
　　CStringData* pOldData = GetData();
　　IAtlStringMgr* pStringMgr = pOldData->pStringMgr; //←ここで止まってしまいます。
　　if( pOldData->nDataLength == 0 )
　　{
　　　return;
　　}

　　if( pOldData->IsLocked() )
　　{
　　　// Don't reallocate a locked buffer that's shrinking
　　　SetLength( 0 );
　　}
　　else
　　{
　　　pOldData->Release();
　　　CStringData* pNewData = pStringMgr->GetNilString();
　　　Attach( pNewData );
　　}
　}

どうかお教え下さい。

回答No.2

Tacosan

「リストの行数分使う」のに「リストの行数分 - 1」だけ確保すれば十分である, という認識ですか?

補足

ごめんなさい、「リストの行数分使う」のに「リストの行数分 - 1」で良いという意味ではなく、私の間違って書いたコードが「リストの行数分 - 1」であって、要修正であるという意味です。「リストの行数」から「－１」しないように、「index＜リストの行数」にしないように訂正しなきゃ駄目ですね。

回答No.1

kmee

他にもあるかもしれませんが、とりあえず
>　array = (struct item*)malloc(sizeof(struct item)*(CFileListCreatorDlg::m_xcList.GetItemCount()-1));

これでstruct itemが何個分、確保できたと思いますか?

>　for (i = 0; i < CFileListCreatorDlg::m_xcList.GetItemCount(); i++) //初期化

これで、何個分、struct itemを使うと思いますか?

それから、mallocに対してのfreeが無いようです。省略したところにあるかもしれませんが。

あとは、C++として使ってますよね?いろいろCには無い機能とか使われてますし。
それなら、malloc/freeよりはnew/delete を使った方がいいと思います。
まず、sizeofを使った計算も、ポインタのキャストも必要ありません。(最近のCならvoid*からキャストする必要は無いですが、C++だと必要)
コンストラクタ/デストラクタを丁寧に書いて、メンバの領域確保/解放を確実にするのもメモリリーク対策の一つですが、malloc/freeではコンストラクタ/デストラクタは呼ばれません。

お礼

目的の機能ができました。malloc/free にしようとしていましたが、new/delete を教えていただきありがとうございます。

Ｖｉｓｔａや７では、ちゃんと描画されるのに、ＸＰでは描画されなかったので、

RedrawWindow(); //追加2011.12.10 //XP対策：再描画

をユーザー関数の最後にいれました。

それでは、またお世話になるかもしれませんが、そのときは宜しくお願いいたします。
（締め切ります。ポイントを差し上げられなくてごめんなさい。）

補足

さっそくのご回答ありがとうございます。お礼が遅くなってすみません。

> >　array = (struct item*)malloc(sizeof(struct item)*(CFileListCreatorDlg::m_xcList.GetItemCount()-1));

> これでstruct itemが何個分、確保できたと思いますか?

構造体を使うのは初めてで、何で型変換やサイズを取るのか、理解はしてませんが、
間違っているのですね。。。リストの行数分 - 1ですね。
でも他が悪いのか、試してみましたが、駄目でした。

> >　for (i = 0; i < CFileListCreatorDlg::m_xcList.GetItemCount(); i++) //初期化

> これで、何個分、struct itemを使うと思いますか?

これは行数分使うと思います。

> それから、mallocに対してのfreeが無いようです。省略したところにあるかもしれませんが。

すみません、投稿するときに削ってしまいましたm(_ _)m

> それなら、malloc/freeよりはnew/delete を使った方がいいと思います。
> まず、sizeofを使った計算も、ポインタのキャストも必要ありません。(最近のCならvoid*からキャストする必要は無いですが、C++だと必要)
> コンストラクタ/デストラクタを丁寧に書いて、メンバの領域確保/解放を確実にするのもメモリリーク対策の一つですが、malloc/freeではコンストラクタ/デストラクタは呼ばれません。

調べてみました、確かにnew/deleteの方が良いようですね。ありがとうございます。
でも、malloc/freeを使った例も知りたいですが…

new/deleteで試みてますが、苦戦中です。
new の宣言部がわかりません・・・

◆開発環境
色々と環境や言語を書いてなくてすみません。
ＶＣ＋＋２０１０、ＭＦＣ、ユニコードを使用する設定になっています。
ＸＰ～７に対応させたいです。関係あるかは分かりませんが、スタティックリンク・マルチスレッドの設定です。