C言語について

2つの文字列を入力して別の文字列にコピーするプログラムをつくっているのですが、どこが間違っているのでしょうか？教えていただけませんか？
/*文字列連結*/
void *my_stract(char*, char*, char*);


void main(void)
{
char bun1[100],bun2[100],bun3[200];
printf("文字列１を入力してください\n"); gets(bun1);
printf("文字列２を入力してください\n"); gets(bun2);

my_stract(bun1,bun2,bun3);

puts(bun3);
}

char *my_stract(char *str3, char *str2, char *str1)
{
char *p1, *p2, *p3;
p1=str1;
p2=str2;
p3=str3;
while(*p1)
{
*p3=*p1;
p1++;
p3++;
}

while(*p2)
{
*p3=*p2;
p2++;
p3++;
}
*p3='\0';
return str3;

}

No.6 回答者： Tacosan 回答日時： 2014/07/09 23:38

「いくらなんでもそれはないだろ」的に巨大な間違いがあるなら, 突っ込んでいいと思いますけどね＞#5.

とよそ向けに書いておくけど, 今までの回答で言われたことを理解できてる?

No.5 回答者： wormhole 回答日時： 2014/07/09 01:25

#1, #2 でもう説明してありますけど

>2つの文字列を入力して別の文字列にコピーするプログラムをつくっているのですが、どこが間違っているのでしょうか？教えていただけませんか？

>char *my_stract(char *str3, char *str2, char *str1)

この関数の説明を書いてみてください。

>my_stract(bun1,bun2,bun3);

その書いた説明の使い方に合っていますか。


#3のコードにも突っ込み入れたい・・・

No.4 回答者： Tacosan 回答日時： 2014/07/08 23:34

あえて突っ込ませてもらいます＞#3.

i < MAX_SRC という比較が全く意味不明. あと, 他の関数にあわせて dst を第1引数においた方がいいとか, src は const 付けてあげた方が親切だろうとか, いくつかありますけどね.

No.3 回答者： syampoooooooooo 回答日時： 2014/07/08 22:48

どこから突っ込めばいいやら…

bun1, bun2, bun3はbuf1, buf2, buf3のスペルミスでは…
my_stractもmy_strcatのスペルミスでは…


それはいいとして…
my_stractの一行目から、
char *p1, *p2, *p3;
p1=str1;
p2=str2;
p3=str3;
は要りません

while(*p1)は、p1がNULL終端してるのかも調べないで回しているので確認する必要があります。while(*p2)も同様
さらに、*p3は100しか容量がないのにただ*p1と*p2をくっつけたら容量をオーバーします。



また、これは定石なのですが、出力が文字列の場合は、戻り値ではなく引数で返るようにするべきです。


ですから、これらを実現するためには、
int my_strcat( char src1[], size_t lenSrc1, char stc2[], size_t lenSrc2, char[] dst, sizeof maxDst );
という形の関数とし、１つ目の文字列、その長さ、２つ目の文字列、その長さ、出力先、その最大長　を渡して、
戻り値には正常終了（例えば0）、出力先の長さが足りなかった（例えば1）、などとするべきです。
（標準のstrcatではこの安全機構がないのでミスを生みやすいです）


参考に、私が書いたソースを載せておきます。
（突貫工事なのでアホなことしてるかもしれません。　丸写しはやめてください。）
http://ideone.com/lXoPII

No.2 回答者： Wr5 回答日時： 2014/07/08 13:29

>char bun1[100],bun2[100],bun3[200];

>my_stract(bun1,bun2,bun3);

で、

>char *my_stract(char *str3, char *str2, char *str1)
>p1=str1;
>while(*p1)

おめでとう。かなり高い確率でバッファオーバーランを実現だ。

mainで確保している【未初期化のbun3】から'\0'が見つかるまで突っ走るぜ！！
俺様を止められるのはOSによる強制停止だげだぜ！ヒッハー！！

ローカル変数はちゃんと初期化しましょうね。
あと、変数名はもう少し気を遣いましょうね。

>while(*p1)
>{
>　*p3=*p1;
>　p1++;
>　p3++;
>}

未初期化のbun3の内容を
>printf("文字列１を入力してください\n"); gets(bun1);
で入力したbun1に対して上書きしていくコードです。
変数名をよ～～っく確認してください。
# char *my_stract(char *str3, char *str2, char *str1)
# の並びで第１引数の変数名が「str3」となっている辺りから誤解がスタートするんです。

No.1 回答者： Tacosan 回答日時： 2014/07/08 13:04

my_stract(bun1,bun2,bun3);

のところ, 何をしてるの?