配列オーバーフロー？

質問

文字列を(順番を変えて)コピーして表示させるプログラムを
作っていますが、うまく表示されずに困っています。

具体的にお話しますと・・・
「abc・・・xyz」→「zyx・・・cba」等としたり、色々
文字列の順序を変えた内容を表示するのでこの部分は問題ないと思ってます。
こうしたプログラムで文字列のchar配列を200程度にして、
1行あたり50文字程度で数行程度ならちゃんと表示されるのですが
これを数十行で行うと部分的に化けるところが出てきます。

わかる人にアドバイスを貰ったところ配列オーバーフローではないか？
と言われましたがそれ以上の情報は得られず、
何が問題なのがよくわからず困っております。

・文字列は一文字ずつ配列を用いている
・文字のコピーはstrcpyを用いている
→for(i=0,j=0;i<MAX;i++)　strcpy(&moji[i],&mojimoji[i]);
・strncpyを用いると「フ」というのが交じってくるので使っていない
・初期化はこんな感じで行っている
→for(i=0;i<MAX;i++) strcpy(&moji[i],"\0");

何よりよくわからないのが、同じような内容の行でも
化ける行と化けない行があり、化けるというのはその行にない文字が
入ってきてしまうというものです。

回答No.10

yaemon_2006

> enc_dec()の先頭にあるこの行は、何をしたいのでしょうか。

　消し忘れです。

回答No.9

asuncion

＞#8さん

> if(!buf) return 0;

enc_dec()の先頭にあるこの行は、何をしたいのでしょうか。
buf が NULL だったら return 0 するって、当該関数で buf[] を定義しているんだから
NULL になるわけがないですよね。

回答No.8

yaemon_2006

#include <stdio.h>
#include <string.h>
#include <stdlib.h>

#define MAX 200

int enc_dec(char *s, int n, int m, int x)
{
char buf[MAX];
int i, j, k;

if(!buf) return 0;
if(x){
for(k = i = 0; i < m; i ++){
for(j = 0; i + m * j < n; ++ j) buf[k ++] = s[i + m * j];
}
}
else{
for(k = i = 0; i < m; i ++){
for(j = 0; i + m * j < n; ++ j) buf[i + m * j] = s[k ++];
}
}
buf[k] = '\0';
strcpy(s, buf);
return 1;
}

int main(int argc, char *argv[])
{
FILE *fp1, *fp2;
char s[MAX], *c;
int x;

if(argc != 4) exit(1);
if(!(fp1 = fopen(argv[1], "r")) || !(fp2 = fopen(argv[2], "w"))) exit(1);
x = atoi(argv[3]);
while(fgets(s, MAX, fp1)){
if(c = strchr(s, '\n')) *c = '\0';
enc_dec(s, strlen(s), 4, x);
fprintf(fp2, "%s\n", s);
puts(s);
}
fclose(fp1);
fclose(fp2);
return 0;
}

回答No.7

D-Matsu

あー、そーゆー仕様な訳ですか。なら配列宣言は問題ないですね。

--
// 暗号化--ここから----4行化
for(i=0,m=0;m<((len/GYOU)+1);m++){
for(n=0;n<GYOU;n++,i++){
strcpy(&code_on[m][n],&moji_before[i]);
if(len==i+1){
n=GYOU;
m=CODE_MAX;
}
}
}
// ----1行化
for(i=0,n=0;n<(GYOU+1);n++){
for(m=0;m<((len/GYOU)+1);m++){
if(strcmp(&code_on[m][n],"\n")!=0){
strcpy(&moji_after[i],&code_on[m][n]);
i++;
}
--

4行化については#6でも触れましたが、
--
i = 0;
for(n=0; len <= (n + 1) * GYOU; n++){
strncpy(&code_on[n][0],&moji_before[n * GYOU]);
code_on[n][GYOU] = '\0';
}
--
の方がスマートですね。
＃つーか#6のコードちょっとバグってるよ＞俺orz

1行化の方ですが、#6でも言った通り「1文字ずつの抽出なら代入を使いましょう」。比較も同様です。
ついでにちとスマート化にチャレンジ。
--
i = 0;
for(n = 0; n < GYOU; n ++){
for(m = 0; m < CODE_MAX; m ++){
if(code_on[m][n] != '\0' && code_on[m][n] != '\n'){ // ちゃんと初期化されていれば余計な文字は入らない
moji_after[i] = code_on[m][n];
i ++;
}else{
// 何もしない
}
}
moji_after[i] = '\0';
}
--

お礼

すいません
スマートなコードにすることでバグが減るというのはわかりますが
根本的な解決には至っておりません

回答No.6

D-Matsu

もう二つばかり気づいたので。
--
char code_on[CODE_MAX][GYOU_IN]; // 4行時の配列
--
これだと長さGYOU_IN(5)、CODE_MAX(51)行の配列、となってしまいます。
正しくは長さCODE_MAX、GYOU_IN行の配列なので
char code_on[GYOU_IN][CODE_MAX];
でなければなりません。
それに合わせて各所の配列の添え字の変更が必要です。

--
// ４行化処理のところ
for(i=0,m=0;m<((len/GYOU)+1);m++){
for(n=0;n<GYOU;n++,i++){
strcpy(&code_on[m][n],&moji_before[i]);
if(len==i+1){
n=GYOU;
m=CODE_MAX;
}
}
}
--
> strcpy(&code_on[m][n],&moji_before[i]);

はstrlen(&moji_before[i])が5-nを超えた場合、code_on[m][n]からcode_on[m][GYOU_IN]に収まりきらず以降のエリアも上書きされます（配列オーバーフロー）。
＃上の指摘前なのでそうなります。上記のように書き換えたら50-nになります

メモリ配置上は次の行の先頭につながっている訳ですが(だから書き換わってしまう)、4行目を書いたあとは未知のエリアに書き込んでいることになるため非常に危険です。

というか#5を書いてる最中にも思ったのですがstrcpy()は「１文字をコピーする関数」ではないということです。そういう目的なら関数なぞ使わず素直に代入を使いましょう。
＃どう見ても使い方がそうなってるので一応念押ししておきます

行あたり何文字をコピーすればいいのかを管理して、strncpy()もしくはmemcpy()を使うべきです。

-- たとえば
i = 0;
cpy_len = (len / GYOU) + 1;
for(n=0;n<GYOU;n++){
strncpy(&code_on[n][0],&moji_before[n * cpy_len]);
code_on[n][n * cpy_len] = '\0';
}
--

補足

最初のだけ。

4行って言い方がまずかったのだとおもいますが、
ABCD
EFGH
IJKL
MN
・
・
・
と4文字ずつ↓に伸びていくのでそのままでよいんじゃないですか？

回答No.5

D-Matsu

--
for(i=0,m=0;i<MAX_IN;m++){
strcpy(&filename_before[i],"\0");
strcpy(&filename_after[i],"\0");
for(n=0;n<GYOU;n++){
strcpy(&code_on[m][n],"\0");
i++;
}
if(i!=MAX_IN){
strcpy(&moji_before[i],"\0");
strcpy(&moji_after[i],"\0");
strcpy(&moji_dummy[i],"\0");
}
}
--
なして配列の0初期化がこんなややこしいコードになるんだろう……
memset()はご存じないですか？
あと、nをカウンタとしたforループの中でiをインクリメントしてる関係で
filename_before,filename_after,moji_before,moji_after,moji_dummyの配列はGYOU(4)間隔でしか0が入りません(初期化漏れ)

これが原因じゃないですか？

お礼

moji_dummyがおかしいっぽいのですがはじめのwhileで初期化してますし・・う～ん・・解決していません。

補足

なにぶん初心者なものでよくわかっていませんです。
初期化漏れですか・・試してみます。

回答No.4

noname#198419

char moji[MAX];
とかで配列の型を宣言してあるならば、「配列の添え字は０からMAX-1までで、MAX個の配列になる」ということが教科書に書いてあるはずです。

for(i=0,j=0;i<MAX;i++)などで、式の評価の順番を教科書などで確認して、forの挙動を観察するための簡単なコードを書いて実験してみると、理解しやすいと思います。

あと、文字列の終わりは'\0'になっているひつようがあることも教科書に書いてあるはずです。

補足

MAX+1にする形にしています。

初めに\0で初期化しています。

全部ではなく、一部分だけが化ける事を不思議に思っています。

回答No.3

arain

No.2です。
とりあえずソースは軽く確認しました。
そこで質問です。

>C言語の初歩の本読んだくらいの知識であれば・・
>ただ、そこから中級者向けとなると理解していないと思います。
Q1.「文字列」の定義を説明してください。
Q2.memcpy()等、mem***()となる関数は知っていますか？

補足

Q1.アスキーコードで表わされる文字が連なるもの、最後に\0がつく
Q2.memmoveは使った事がありますが、これでも「フ」が出ました。
また、具体的な事はよくわかっていません。

この程度ですかね

回答No.2

arain

全部のソース公開してください。

>・strncpyを用いると「フ」というのが交じってくるので使っていない
から、確実に配列の定義や使用方法、str系の関数の使い方が間違っていることはわかりますが、それ以上のことはわかりません。

ちなみに、C言語の「文字列の定義」って理解されてます？

お礼

＃１さんへのコメント追記です。
全て半角文字です。OSはVISTA、Visual C++6.0用いてます。

補足

C言語の初歩の本読んだくらいの知識であれば・・
ただ、そこから中級者向けとなると理解していないと思います。

回答No.1

tatsu99

まず、ソースを提示して下さい。それと文字列中に漢字（２バイトの文字）は、ないですよね。あと、コンパイラ＆ＯＳも提示されていると、良いでしょう。

お礼

// dummyに一時保存
for(i=0;i<len_sub;i++) strcpy(&moji_dummy[i],&moji_after[i]);
for(i=0,j=0;i<len;i++){
if(strcmp(&moji_after[i],"\0")==0){
j++;
}
strcpy(&moji_after[i],&moji_dummy[i+j]);
}
strcpy(&moji_after[i],"\n");

if(fputs(moji_after, fp_after)==EOF){
printf("書き込み失敗\n");
return -1;
}
}

fclose(fp_after);
fclose(fp_before);

return 0;
}

やっていることは、
ファイルopen→（1行分の文字列read→暗号化→ファイルにwrite）繰り返し→ファイルcloseのつもりです。
ここでいう暗号化とは「abcdefghijklmn」を以下のように4行に分けて
abcd
efgh
ijkl
mn
横読みだったものを縦読みにし、「aeimbfjncgkdhl」とします。

どうぞよろしくおねがいします。

補足

#include<stdio.h>
#include<string.h>
#define MAX 200 // 1行最大文字数
#define MAX_IN MAX+1 // 1行最大文字数+\0
#define GYOU 4 // 暗号途中処理の1列4文字まで
#define GYOU_IN GYOU+1 // 暗号途中処理の1列4文字まで+\0
#define CODE_MAX MAX/GYOU+1 // 暗号途中処理の1行の最大長+\0
int angou(void);
int kaidoku(void); /* 未作成 */

int main(){
int ret=1;
ret=angou();
return ret;
}
/******************** 暗号化 ********************/
int angou(){
int i,j,m,n,len,len_sub;
FILE *fp_before; // 前ファイルポインタ
FILE *fp_after; // 後ファイルポインタ
char filename_before[MAX]; // 前ファイル名
char filename_after[MAX]; // 後ファイル名
char moji_before[MAX_IN]; // 前文字列
char moji_after[MAX_IN]; // 後文字列
char moji_dummy[MAX_IN]; // 文字列交換用
char code_on[CODE_MAX][GYOU_IN]; // 4行時の配列
char *fg; // fgets用戻り値

/***** 初期化処理 *****/
for(i=0,m=0;i<MAX_IN;m++){
strcpy(&filename_before[i],"\0");
strcpy(&filename_after[i],"\0");
for(n=0;n<GYOU;n++){
strcpy(&code_on[m][n],"\0");
i++;
}
if(i!=MAX_IN){
strcpy(&moji_before[i],"\0");
strcpy(&moji_after[i],"\0");
strcpy(&moji_dummy[i],"\0");
}
}

/***** 暗号化前ファイル名入力 *****/
printf("\n＠＠＠＠＠暗号化前ファイル名入力\n");
scanf("%s",&filename_before);
// 暗号化前ファイルオープン
if((fp_before=fopen(filename_before,"r"))==NULL){
printf("ファイルがありません\n");

return -1;
}

/***** 暗号化後ファイル名入力 *****/
printf("\n＠＠＠＠＠暗号化後ファイル名入力\n");
scanf("%s",&filename_after);
// 暗号化後ファイルオープン
if((fp_after=fopen(filename_after,"r"))!=NULL){
printf("同名のファイルが既にあります\n");

return -1;
}
fp_after=fopen(filename_after,"w");

/***** ファイルから全文取り出す *****/
while(1){

for(i=0;i<MAX_IN;i++){
strcpy(&moji_before[i],"\0");
strcpy(&moji_after[i],"\0");
strcpy(&moji_dummy[i],"\0");
}

fg=fgets(moji_before,MAX_IN,fp_before);
if(fg==NULL){
break;
}

len=strlen(moji_before)-1;

if(len%GYOU!=0){
len_sub=len+(GYOU-len%GYOU);
}
else len_sub=len;

// 暗号化--ここから----4行化
for(i=0,m=0;m<((len/GYOU)+1);m++){
for(n=0;n<GYOU;n++,i++){
strcpy(&code_on[m][n],&moji_before[i]);
if(len==i+1){
n=GYOU;
m=CODE_MAX;
}
}
}
// ----1行化
for(i=0,n=0;n<(GYOU+1);n++){
for(m=0;m<((len/GYOU)+1);m++){
if(strcmp(&code_on[m][n],"\n")!=0){
strcpy(&moji_after[i],&code_on[m][n]);
i++;
}
}
if(len==i+1){
// strcpy(&moji_after[i],"\n");
n=GYOU;
m=CODE_MAX;
}
}
// 暗号化--ここまで--------------------------------------