エラーがないのに正常に動かない

質問

以前質問したのものです。
アドバイス等いただき、プログラムを修正しエラーをなくしました。
しかし、プログラムが動きません。。。

プログラムの動作は
CSVファイルを読み込んで、処理をする。
CSVファイルは
単語1,数値データ
単語2,数値データ
のようになっており、
これをsの配列に格納したいと思っています。

プログラムの内容は
#include<stdio.h>
#include <string.h>

int main(void)
{
FILE *fp;
char s[5][1000][2][256];
char tp[256];
char *ttp;
int i=0,ii=0;

if((fp=fopen("out1.csv","r"))==NULL){
printf("ファイルオープンできませんよ\n");
return -1;
}

while(fgets(tp,256,fp)!=NULL){
ttp=strtok(tp,",");
strcpy(&s[0][ii][i][0],ttp);
puts(&s[0][ii][i][0]);
while (ttp != NULL ) {
i++;
ttp=strtok(NULL,",");
if (ttp != NULL ){
strcpy(&s[0][ii][i][0],ttp);
puts(&s[0][ii][i][0]);
}}i=0;
ii++;if(ii>=1000)break;
}

return(0);
}
です。
わけがわからなくなりました（泣
どこが原因なのかも書いていただけるとありがたいです。

回答No.4

noname#144013 · ベストアンサー

ionwide さん、こんにちは。

■まず、問題点を上げます。
１）スタックサイズを大きくとる必要がある。
　※これは、【ANo.1】の koko_u_ さんが仰っていることと関連があります。

　配列 s[5][1000][2][256] が大きいので、コンパイルまたはリンク時のオプションで
　スタックサイズの指定を行い、そのサイズを大きく採っておく必要があります。
　コンパイラによりますが、デフォルトだと大体１ＭＢぐらいしか採られないので、デフォ
　ルトのままコンパイルして実行するとおそらく異常が発生します。
　今回のソースの場合、配列が約２．５ＭＢになるので、スタックサイズは約４ＭＢぐらい
　採った方が安全だと思います。
　それと、koko_u_ さんも仰っているように、この配列構造だと使われない無駄なエリアが
　発生しますので、koko_u_ さんの示されているように、構造体の配列に変更された方が
　良いかもしれません。
　※構造体にすることでデータのタイプ（文字列、数値等）を明確にできますし、後の処理
　　が楽になると思います。

２）２つ目のWhileループがいらない。
　※これは、【ANo.3】の redfox63 さんが仰っていることと同様な事柄です。

　配列 s[5][1000][2][256] の３次元目の配列数が ２ となっているのに、このループを通す
　ことで配列の上限を超えてアクセスすることになり、異常が発生する可能性があります。
　CSVファイル内のデータ記述が２列の場合は問題ないですが、３列以上だった場合は、
　問題となります。

３）CSVデータ取り出し用のデリミタにCR/LFを追加した方が良い。
　fgets関数で１行テキストを取り出した場合、末尾の改行文字（CR/LF）は削除されずに
　取り出した文字列に含まれます。
　そのため、ttp=strtok(NULL,",") で抽出した文字列にも改行文字が含まれてしまいます。
　余分な改行文字を含めたくない場合は、デリミタ文字列を ",\r\n" とした方が良いと思わ
　れます。

４）ファイルクローズをしていない。
　CSVファイルの読み出しが終わった後に、ファイルをクローズしていないので、オープン
　したままとなり、他のアプリケーションなどからそのCSVファイルをアクセスしようとした
　場合にアクセスできない可能性があります。
　ひと通りファイルの処理が終了したら、ファイルをクローズした方が良いと思います。

■修正バージョン
以上の問題点を修正したバージョンを下記に上げてみました。
　・ソース中の//##～～の行は、オリジナルソースの修正前の記述です。
　・配列構造はそのままにしてあります。
参考にして頂ければ幸いです。
※なお、下記ソースはあくまで一例であり、これがベストな方法ではないので、
ionwide さんご自身でいろいろと対処法・改良点などを考えてみてください。

/*
* getcsv.cpp：CSVファイル読込みサンプルプログラム（修正版）
*/
#include <stdio.h>
#include <string.h>

int main(void)
{
FILE *fp;
char s[5][1000][2][256]; //←※この配列の為にスタックサイズ指定を大きくする（4MBぐらい？）
char tp[256];
char *ttp = NULL;
int i=0,ii=0;

if((fp=fopen("out1.csv","r"))==NULL){
printf("ファイルオープンできませんよ\n");
return -1;
}

while(fgets(tp,256,fp)!=NULL)
{
//##ttp=strtok(tp,","); //←※変更（デリミタにCR/LFを追加）
ttp=strtok(tp,",\r\n"); //←　〃
strcpy(&s[0][ii][i][0],ttp);
puts(&s[0][ii][i][0]);
//##while (ttp != NULL ) { //←※削除（このループはいらない）
i++;
//##ttp=strtok(NULL,","); //←※変更（デリミタにCR/LFを追加）
ttp=strtok(NULL,",\r\n"); //←　〃
if (ttp != NULL ){
strcpy(&s[0][ii][i][0],ttp);
puts(&s[0][ii][i][0]);
}
//##} //←※削除
i=0;
ii++;if(ii>=1000)break;
}
fclose(fp); //←※追加（ファイルクローズ）
return(0);
}

回答No.6

D-Matsu

＃「プログラムは思った通りには動かない。書いた通りに動く」という有名な言葉がありまして……

コード自体については概ね他の方が書かれているので、ちょっと前回の回答の補足のようなものを。

前回の質問：http://okwave.jp/qa4340465.html

前回の回答では元コードの修正が少なくなるような形で書きましたが、すでに言われている通り本来は構造体を使うべき処理です。
また数値はint、float等の数値型で持つのが一般的です。
あと、意味のある変数はちゃんと名前をつけておくと後々わかりやすいですよ。

-- example --
typedef struct _tagDatum {
char word[32]; // 実際こんなもんあれば問題ないでしょう
int value;
} datum;
datum csv_data[5][1000];
-- example --

前回の最後にもさらっと触れましたが、データフォーマットが決まっている場合はfscanf()が便利です。
＃fgets()で取ったデータをsscanf()でもOK

-- example --
int elem = 0;
int array_ptr = 0;
while((fscanf(fp, "%s,%d\n", &(csv_data[0][elem].word[0]), &(csv_data[0][elem].value)) != 2) && (elem < 1000))
{
elem ++;
}
-- example --

回答No.5

php504

構造体を使ったことが無いのでしょうか
これは構造体を使うべき処理なのでこの機会にぜひ#1さんの書かれたような構造体を使った処理を覚えましょう

回答No.3

redfox63

> しかし、プログラムが動きません
では 何に困っているかが理解できないのですが …

期待する結果と実行結果とその元になるデータを示しましょう

数値データが可変個数なのでしょうか？
3次元目は2つしか確保していないのに2番目3番目のデータが来ると予期しない配列の部分(またはメモリー位置)を書き換えてしまいます

単語,100
といった行のの処理なら このsの宣言でいいのですが
単語,100,200,300
単語,150
単語,60,200
などの 可変個数データには対処不能ですよ

回答No.2

asuncion

当該４次元配列の、各次元に関する説明をお願いいたします。

お礼

s[１番目][２番目][３番目][４番目]とすると、
２番目は１０００行のデータを入力するための配列
３番目は単語か、数値データのどちらかを決める配列
４番目は単語または数値データが入る配列

ちなみに１番目はこの配列を５つ用意するつもりでした。

回答No.1

koko_u_

えーと。この 4次元配列じゃないとダメなんっすかねえ？

普通に CSV ファイルの 1行を

struct csv_data_tag {
　char word[WORD_LENGTH];
　int value;
};

みたいにして欲しい。