2012-04-26 3 views
-3

マップに4つの文字列を格納して初めて印刷する次のプログラムがあります。今度は、別の時間を実行して保存された値を取得します。しかし、2回目のリザルトは初めての結果と同じではありません。マップを持つC++プログラムは、前回格納されたものと同じ再割当てをしていません。

#include <map> 
using namespace std; 

void fun_call(void **,char *); 
main(){ 
     void *data=NULL; 
     char value[100]; 
     int i=0,j=0; 

     char key[][10]={"disk1","disk2","disk3","disk4"}; 

     cout << "printing all mapped values " << endl ; 
     data = (void *) malloc(100); 

     for(j=0;j<2;j++){ 
     for(i=0;i<4;i++){ 
       fun_call(&data,key[i]); 
       memcpy(value,data,100); 
       cout << "key ="<<key[i]<<" value is " << value << endl; 
     } 
     cout <<"====================="<< endl; 
     } 
} 

void fun_call(void **tmp,char name[10]) 
{ 
     void *tmp_data; 
     char str[100]="ravindra"; 
     int len =0; 

     static std::map<std::string,void *> name_data_map; 
     std::map<std::string,void *>::iterator iter ; 

     iter=name_data_map.find(name) ; 

     if (iter == name_data_map.end()) 
     { 
       len=strlen(str)+strlen(name)+1; 
       tmp_data = (void *) malloc (len); 
       strcat(str,name); 
       memcpy(tmp_data,str,len); 
       name_data_map[name]=tmp_data; 
       cout << "Inside the if" << endl ; 
     } 
     else 
       cout << "disk pos "<< iter->first << endl; 
     cout << "Outside the if" << endl ; 
     iter=name_data_map.find(name) ; 
     memcpy(*tmp,iter->second,len); 

} 

出力:

 
$ ./a.out 
printing all mapped values 
Inside the if 
Outside the if 
key =disk1 value is ravindradisk1 
Inside the if 
Outside the if 
key =disk2 value is ravindradisk2 
Inside the if 
Outside the if 
key =disk3 value is ravindradisk3 
Inside the if 
Outside the if 
key =disk4 value is ravindradisk4 
===================== 
disk pos disk1 
Outside the if 
key =disk1 value is ravindradisk4 
disk pos disk2 
Outside the if 
key =disk2 value is ravindradisk4 
disk pos disk3 
Outside the if 
key =disk3 value is ravindradisk4 
disk pos disk4 
Outside the if 
key =disk4 value is ravindradisk4 

2回目の繰り返しのように、すべてのデータを与えている理由を任意のアイデア:中

+7

このメモリコピーとポインタ演算は、すべて非常に難しいです。定義されていない動作を呼び出す可能性は非常に高いですが、気づいていません。 std :: stringを使用して、問題がなくならないか再度確認することを検討してください。 – thiton

+0

合意。私は問題は、あなたが割り当てた後に 'memset'を使って配列を0に初期化しなかったという事実から来ていると思います。 STLコンテナとCスタイルの文字列を混在させるのは悪い習慣です。 –

+3

文字列を扱うときに、なぜvoid *をマップに格納していますか?ここでやったようにタイプセーフな作業ができない理由はありますか?http://stackoverflow.com/questions/10333484/c-program-with-map-explain-the-below-program-how-its-working/10334625#10334625? – stefaanv

答えて

3

lenfun_callの初めに0に設定されている "ravindradisk4"、そうならば2回目は実行されませんifmemcpy最後に0バイトをコピーします。したがって、最初の反復からの最後のvaluemain()は、keyにかかわらず同じままです。

+0

で説明されているような良い例を表示してください。ありがとうございます – user1044923

0

まず第一には、一般的には、C++new/delete代わりのmalloc()/free()を使用することを検討してください。

正確に何を達成しようとしているのか(つまり値を連続してコピーする理由)はわかりませんが、長さは設定されていません。memcpy()は何もコピーしません。

iter->secondに保存されているポインタを使用することもできます(dataを変更してそのマップエントリを更新することができます)。例えば

このラインをメインにご data変数のメモリを割り当て、単に変更されません
memcpy(*tmp, iter->second, len); 

*tmp = iter->second; 

に今メインでdataのポインタアドレスをポインタに設定されていますマップに格納されているアドレス。

1

有効な(またはリモートで慣用的な)C++プログラムを意図している場合、コードには多くの問題があります。

@starbugsが指摘するように、結果をコピーするために2回目の正しい長さを使用していません。

memcpy(*tmp,iter->second,len); 

...へ:ワンライン「修正」を変更するだろう脆性C文字列の技術は最高のC++の方法論に置き換えている理由のいくつかの基本については

memcpy(*tmp,iter->second,strlen((char*)iter->second)+1); 

、私がしたいですあなたがC++標準ライブラリが使用されるべきで精神を採用する方ができるかもしれないことを把握したらBjarne

によって

Learning Standard C++ As A New Language (PDF):人々にこれを示します。

あなたのプログラムは、はるかに堅牢で読みやすい慣用的なコードを生成するために単純化することができる方法を示すために簡単だように簡単です:

#include <map> 
#include <iostream> 
#include <string> 

using namespace std; 

string fun_call(string name) 
{ 
    static map<string,string> name_data_map; 

    map<string,string>::iterator iter; 
    iter = name_data_map.find(name); 

    if (iter == name_data_map.end()) { 
     string mapvalue = "ravindra"; 
     mapvalue += name; 
     name_data_map[name] = mapvalue; 
     cout << "Inside the if" << endl ; 
    } 
    else 
     cout << "disk pos "<< iter->first << endl; 

    cout << "Outside the if" << endl; 
    iter = name_data_map.find(name) ; 
    return iter->second; 
} 

int main() { 
    string keys[] = {"disk1","disk2","disk3","disk4"}; 

    cout << "printing all mapped values " << endl ; 

    for(int j = 0; j < 2; j++) { 
     for(int i = 0; i < 4; i++){ 
      string value = fun_call(keys[i]); 
      cout << "key =" << keys[i] <<" value is " << value << endl; 
     } 
     cout << "=====================" << endl; 
    } 
} 

私は基本的に同等のものを提供することが停止されます同じ出力と制御フローを持つプログラム。

注:標準C++で

  • (それは奇妙なことに、引数またはreturn文は必要ありませんが)、メインは、戻り値の型としてint型を持っている必要があります

  • using namespace std;ライン文字列、マップ、イテレータなどの標準ライブラリクラスの前に、std::と入力する必要がなくなります。しかし、それを含む他のソースファイルに問題を引き起こす可能性があり、曖昧さがない場合に標準名と矛盾するかもしれない独自の定義を持つため、ヘッダーファイルに入れないでください。

  • 標準ライブラリを使用すると、値の型がメモリ管理を行い、使用するメモリがクラス内で割り当てられ、デストラクタで解放されます。明示的なメモリ管理を行う必要がありますか?

+0

私は何とかあなたのコードで間違いを見つけるのが好きでしたが、私は@HostileForkここに。また、ここで静的なマップを使用しないでください。 – starbugs

0

まず、コードのコンパイル方法がわかりません。あなたの主な関数は戻り値の型がなく、void/no-returnはちょうど悪い習慣です。単純な戻り値0に対応するように再構成し、戻り値の型をintにします。

さらに、コンパイルする前にいくつかのインクルードが欠落しています(つまり、iostreamと文字列)。 using namespace stdを使用する代わりに、必要なものだけをstd名前空間から「プル」しようとします。将来的に命名規則の衝突に遭遇する可能性がある(そして頭痛を引き起こす可能性がある)ので、それをすべて取り入れることは潜在的な危険と悪い習慣です。

手元の問題に戻ってください。あなたは、あなたの心を実験したり、罰したりしていないなら、ここでいくつかの非常に悪い習慣を適用しています。このような多くのメモリコピーやポインタの移動は、頂点バッファを動かす作業をしている間でさえ得られません。あなたの割り当てと割り当てを一致させると、これは非常に悪いメモリ管理です。 C++ではnew/deleteを使用しています。

データ変数へのポインタのアドレスを渡すので、単に* tmpを使ってデータのポインタを変更できます。

あなたのname_data_mapは静的なので、ループに残っています。したがって、iterの2番目のデータメンバは、現在のデータオブジェクトへの実際のポインタです。単にあなたの第二の機能のコードの最後の行を変更:とにかく

*tmp = iter->second; 

を、それは私の2セントだ...私も、あなたが何をしようとして得ることはありません。がんばろう!

+0

いくつか良い点を挙げていますが、これは問題の解決策ではありません。それは部分的に動作しますが、実際にはOPの問題は悪い習慣で詰まっていると言われています。静的なマップが実際に飛び出します。 –

+0

@DomagojPandžaうん、まあ、これは私には全く新しいものです。うまくいけばそれはちょっと役立ちます。 –

関連する問題