Go to list of users who liked
Share on X(Twitter)
Share on Facebook
More than 5 years have passed since last update.
C言語はもうかれこれ10年くらい書いていないけど、流石にこれはヤバい。
正直な感想として、ブランド毀損するくらいの危険性をはらんでいると思う。
当該記事からコピーしてきた。
# include <stdio.h># include <stdlib.h>// 構造体の宣言typedefstruct{intnum;char*str;}strct;intmain(void){// 実体を生成strct*entity;// 動的メモリの確保。確保したメモリをstrct型ポインタにキャスト。entity=(strct*)malloc(sizeof(strct));// メンバの初期化entity->num=0;entity->str=(char*)malloc(sizeof(32));// メモリに文字列を代入sprintf(entity->str,"%s %s!","Hello","World");printf("%s\n",entity->str);// メモリの解放free(entity->str);free(entity);return0;}まず気になるのが、このコメント
// 実体を生成strct*entity;実体というのは、どういう意味で書いたんだろう。
これはただのポインタなんだけど・・・。
個人的には、変数の宣言と同時に初期化するのが好きなので、私ならこう書くと思う。
strct*entity=(strct*)malloc(sizeof(strct));この辺は、まだいい。
驚愕のコードがここだ。
entity->str=(char*)malloc(sizeof(32));sizeof(32)が何を返すか理解していないと思う。
32はint型だ。
これはsizeof(int)と同じで、処理系にもよるけど、大抵のパターンは4が返ってくるんじゃないかと思う。
つまり、4バイト分のメモリしか確保していないんですけど、大丈夫？（大丈夫じゃない）
仮に32文字分のメモリを確保したいとしたら、
entity->str=(char*)malloc(sizeof(char)*32);と書けば良いかと思う。
あと、メモリ確保に失敗することもあるので、戻り値のNULLチェックはしたほうがいい。いや、するべきだ。
// メモリに文字列を代入sprintf(entity->str,"%s %s!","Hello","World");怖くて、正直こんなコードはなかなか本番じゃ書かないと思うけど、バッファオーバーフローの危険性をはらんでいる。
実際、初期のコードだとバッファオーバーフローしている。
このコードについては、これくらいにして、次のコード。
# include <stdio.h># include <stdlib.h># include <string.h>// 構造体の宣言typedefstruct{intnum;char*str;}strct;intmain(void){// 実体を生成strct*entity;// 動的メモリの確保。確保したメモリをstrct型ポインタにキャスト。entity=(strct*)malloc(sizeof(strct));// メンバの初期化entity->num=0;entity->str=(char*)malloc(sizeof(32));// メモリに文字列を代入sprintf(entity->str,"%s %s!","Hello","World");printf("%s\n",entity->str);chararr_str[]="Hello USA!";// メモリサイズの変更entity->str=(char*)malloc(sizeof(arr_str));if(entity->str==NULL){printf("memory error");return-1;}// アドレスの先頭からarr_strのバイト数分だけNULLで書き換えmemset(entity->str,'\0',sizeof(arr_str));printf("%s\n",entity->str);// メモリの解放free(entity->str);free(entity);printf("processing completion\n");return0;}sizeof(32)はさっき指摘したので省略するとして、
// メモリサイズの変更entity->str=(char*)malloc(sizeof(arr_str));おーい、これもともとあったentity->strのメモリはどうするんだよ。
立派なメモリリークの誕生だよ。
次！
# include <stdio.h># include <stdlib.h># include <string.h>// 構造体の宣言typedefstruct{intnum;char*str;}strct;intmain(void){// 実体を生成strct*entity;// 動的メモリの確保。確保したメモリをstrct型ポインタにキャスト。entity=(strct*)malloc(sizeof(strct));// メンバの初期化entity->num=0;entity->str=(char*)malloc(sizeof(32));// メモリに文字列を代入sprintf(entity->str,"%s %s!","Hello","World");printf("%s\n",entity->str);//構造体の実体のコピーstrct*copy_entity;copy_entity=(strct*)malloc(sizeof(strct));memcpy(copy_entity,entity,sizeof(strct));// メンバのポインタは浅いコピー// 浅いコピーのため、コピー元の値が変わるとコピー先の値も変わるstrcpy(entity->str,"Hello Japan!");printf("%s, %s\n",entity->str,copy_entity->str);// 深いコピーにするためには、メンバ個別でコピーが必要copy_entity->str=(char*)malloc(sizeof(32));strcpy(copy_entity->str,entity->str);// 深いコピーのため、コピー元の値が変わってもコピー先の値は変わらないstrcpy(entity->str,"Hello USA!");printf("%s, %s\n",entity->str,copy_entity->str);// メモリの解放free(copy_entity->str);free(copy_entity);free(entity->str);free(entity);return0;}sizeof(32)は・・・（省略）
memcpy(copy_entity,entity,sizeof(strct));// メンバのポインタは浅いコピー悪いとは言わないけど、このパターンなら、*copy_entity = *entityって私なら書くかな。
memcpyのサンプルにしたいなら、構造体の配列くらい用意してもいいかもね。
最後！
# include <stdio.h># include <stdlib.h># include <string.h>// 構造体の宣言typedefstruct{intnum;char*str;}strct;intmain(void){// 実体を生成strct*entity;// 動的メモリの確保。確保したメモリをstrct型ポインタにキャスト。entity=(strct*)malloc(sizeof(strct));// メンバの初期化entity->num=0;entity->str=(char*)malloc(sizeof(32));// メモリに文字列を代入sprintf(entity->str,"%s %s!","Hello","World");printf("%s\n",entity->str);//構造体の実体のコピーstrct*copy_entity;copy_entity=(strct*)malloc(sizeof(strct));memcpy(copy_entity,entity,sizeof(strct));// メンバのポインタは浅いコピー// コピー元とコピー先を比較演算if(memcmp(entity,copy_entity,sizeof(©_entity))==0){printf("構造体の実体%sと%sは同じです\n","entity","copy_entity");}else{printf("構造体の実体%sと%sは別です\n","entity","copy_entity");}// 深いコピーにするためには、メンバ単体でコピーが必要copy_entity->str=(char*)malloc(sizeof(32));strcpy(copy_entity->str,entity->str);// コピー元とコピー先を比較演算if(memcmp(entity,copy_entity,sizeof(©_entity))==0){printf("構造体の実体%sと%sは同じです\n","entity","copy_entity");}else{printf("構造体の実体%sと%sは別です\n","entity","copy_entity");}// メモリの解放free(copy_entity->str);free(copy_entity);free(entity->str);free(entity);return0;}このコード大丈夫？（だから、大丈夫じゃないって）
// コピー元とコピー先を比較演算if(memcmp(entity,copy_entity,sizeof(©_entity))==0){printf("構造体の実体%sと%sは同じです\n","entity","copy_entity");}else{printf("構造体の実体%sと%sは別です\n","entity","copy_entity");}特に、sizeof(©_entity)この意味理解してる？
copy_entityの型ってstrct *でしょ？それに&をつけると、どうなるの？
はい！strct **型を表します。
で、そのsizeofってことは？？？
・・・
処理系にもよるけど、大抵の場合、4か8が返ってくると思うよ。
つまり、構造体のint numに相当する部分しか比較していないと思うんだけど・・・。
もし書き直すとしたら、sizeof(strct)かな。
間違ってもsizeof(copy_entity)じゃないからね！
ついでに言うと、パディング云々言ってるわけだから、構造体もメモリ確保した後、memset使って初期化したほうがいいんじゃないかな？
正直なところmemcmpで比較するんじゃなくて、比較用の関数を作ってメンバの中身までをチェックしたほうがいいと思う。C++なら=演算子のオーバーロードだね。
最後に・・・
間違いは誰にでもある。
コード憎んで、人を憎まず。
技術系の記事って検証に検証を重ねないと怖いですね。
Register as a new user and use Qiita more conveniently
- You get articles that match your needs
- You can efficiently read back useful information
- You can use dark theme