このページは、CQAに来た質問の回答を考える仮定を、 ほとんどそのままメモ的な感じで書きとめてみたものです。 プログラミングの途中の思考がどうなっているか知りたい方へのヒントになるかもしれないと思いまして、 あえて公開を試みています。 基本的に独り言ということで、 表現がぞんざいになりがちですが、その点はご了承ください。
10人分の住所録（コード名、名前、住所、電話番号、メールアドレス）を作成して、 保存と編集をしたい。 保存したファイルを見ると文字化けが起こってきちんと表記されない。
なお、 低水準関数を勉強中なので、それらを使いたいという条件が付いている。
元のコード
#include <sys/types.h>
#include <sys/stat.h>
#include <fcntl.h>
#include <unistd.h>
#include <string.h>
#include <ctype.h>
#include <stdlib.h>
typedef struct address{
int code;
char name[20];
char add[50];
char tel[15];
char mail[50];
}add;
int main(){
add a;
int flgs =O_TRUNC | O_CREAT | O_RDWR;
int mode = 00744;
int i,fd,flg;
add addtable[10];
int bufnum;
int load(add *);
char filename[]="data1";
if((fd =open("data1",flgs,mode))==-1){
printf("open error!\n");
exit(-1);
}
printf("1〜10人までのコードを入力できます。(終了時はコード0）\n");
for(i=1;i<11;i++){
printf("\n code ?");
scanf("%d",&a.code);
if(isalpha(a.code) || a.code>10 || a.code<1){
printf("１〜１０の数字をコードに入力してください\n");
break;
}
printf("Name ?");
scanf("%s",a.name);
printf("address?");
scanf("%s",a.add);
printf("TEL.No ?");
scanf("%s",a.tel);
printf("mail.No ?");
scanf("%s",a.mail);
write(fd,&a.code,sizeof(int));
write(fd,a.name,20);
write(fd , a.add , 50);
write(fd,a.tel,15);
write(fd,a.mail,50);
flg = read(fd,&a.code,sizeof(int));
flg = read(fd,a.name,sizeof(a.name));
flg = read(fd,a.add,sizeof(a.add));
flg = read(fd,a.tel,sizeof(a.tel));
flg = read(fd,a.mail,sizeof(a.mail));
if(flg<0){
printf("read error\n");
break;
}
printf("\n code : %d\n",a.code);
printf(" name : %s\n",a.name);
printf(" address : %s\n",a.add);
printf(" TEL.No : %s\n",a.tel);
printf(" mail.No : %s\n",a.mail);
}
close(fd);
printf("これからなにをしますか\n");
printf("1 : load , 2 : quit\n");
scanf("%d" ,&bufnum);
switch(bufnum){
case 1 :
load(addtable);
break;
default:
break;
}
}
int load(add *pt){
int fd;
int i;
char filename[20];
printf("保存したファイルを開きます。\n");
printf("開きたいファイル名を指定してください\n");
scanf("%s" , filename);
fd = open("filename", O_RDONLY);
read(fd , pt , sizeof(add)*10 - 1);
for(i = 0 ; i < 10 ; i++){
printf("%d\n" , (pt+i)->code);
printf("%s\n" , (pt+i)->name);
printf("%s\n" , (pt+i)->add);
printf("%s\n" , (pt+i)->tel);
printf("%s\n" , (pt+i)->mail);
}
close(fd);
}
【考察】
まず、低水準関数を使うというのはかまわないが、 使い方がおかしい。 というか、使う上での発想がおかしい。 わざわざ「低水準」というような名前が付いているのは、 特別な理由がない限り、 それを使って高水準関数を作ることが想定されていると考えるべきである。 でも想定されていないかも。 まあいいか。 どうしても直接使うというのなら、 scanfとかprintfを使うというのが矛盾している。
その前にネーミングがおかしいのですが。 コード名、名前、住所、電話番号、メールアドレス、 を入れる構造体がこうなっている。
typedef struct address{
int code;
char name[20];
char add[50];
char tel[15];
char mail[50];
}add;
とりあえず、add では何のことか分からないので、次のように修正。
typedef struct {
int code;
char name[20];
char address[50];
char tel[15];
char mail[50];
} address_struct;
addだと、加算のことか、とか勘違いしてしまう。 どうしても長いのがいやなら addr にすればいい。 それにしても、 int code に「コード名」は入らないのでは? というか、コード名って何? スパイ大作戦に対応するとか、そういう話なのだろうか。 住所録にコード名という欄があるのが当たり前だったら説明の必要もないのだが、 もしかして世間一般常識がないですか、私?
ひょっとして郵便番号のことか? だったらintに入るかどうか怪しいものだが…。 とか思っていた。この時点では。
基本ですが、この種の定数を直書きするのはよくないというのも常識だが、 ではマクロで定義するのか、使える所は sizeof にするのか、 というあたりで少し悩むかも。 ということで、そこも修正するとして。 まず、低水準関数を使った高水準の入り口を書く。 これはそんなに大それた話ではなくて、 単にそういう関数にまとめるだけ。
void write_address(int fd, address_struct address)
{
write(fd, &address.code, sizeof(int));
write(fd, address.name, sizeof(address.name));
write(fd, address.add , sizeof(address.add));
write(fd, address.tel, sizeof(address.tel));
write(fd, address.mail, sizeof(address.mail));
}
元のコードは定数直書だったのを直してある。 マクロにするという手はあるが、 ここは sizeof の出番だろう。 ついでに、write するのだから read も作る。
void read_address(int fd, address_struct *p_address)
{
read(fd, &(p_address->code), sizeof(int));
read(fd, p_address->name, sizeof(p_address->name));
read(fd, p_address->add , sizeof(p_address->add));
read(fd, p_address->tel, sizeof(p_address->tel));
read(fd, p_address->mail, sizeof(p_address->mail));
}
エラーがあったらどうなる? とりあえず値返すか。
int read_address(int fd, address_struct *p_address)
{
if (read(fd, &(p_address->code), sizeof(int)) < 0)
return -1;
if (read(fd, p_address->name, sizeof(p_address->name)) < 0)
return -1;
if (read(fd, p_address->add , sizeof(p_address->add)) < 0)
return -1;
if (read(fd, p_address->tel, sizeof(p_address->tel)) < 0)
return -1;
if (read(fd, p_address->mail, sizeof(p_address->mail)) < 0)
return -1;
return 0;
}
高水準というにはちょっと低機能すぎるので、 中水準関数って所か。 これらの中水準関数を使って、元のコードを書き換える。 と思ったが、まてよ。
write(fd,&a.code,sizeof(int));
write(fd,a.name,20);
write(fd , a.add , 50);
write(fd,a.tel,15);
write(fd,a.mail,50);
flg = read(fd,&a.code,sizeof(int));
flg = read(fd,a.name,sizeof(a.name));
flg = read(fd,a.add,sizeof(a.add));
flg = read(fd,a.tel,sizeof(a.tel));
flg = read(fd,a.mail,sizeof(a.mail));
if(flg<0){
printf("read error\n");
break;
}
writeは数値直接指定でreadはsizeofというのも面妖だが、 fdをこういう使い方していいのか? というか、 ファイルはO_TRUNCでオープンしているから上書きされ、 最初はサイズ0である。 ここでwriteで何か書き込むのはいいが、 その後そのままreadしたら何が出てくるのだろう? このデータ構造のサイズがいくつか知らないが、 仮に500バイトだとして、 write でデータを書き込んだら、ファイルポインタ(謎)は、 500バイト目のところを指しているから、 続けて read すると、 500バイト目からその後の500バイトを読み込もうとするのでは? 何となく read のエラー処理がおかしいような気がしたので、 読み込んだサイズもチェックするように修正。
ちなみに、ここまできてやっと「コード名」というのが何なのか、 何となく分かってきたような気がする。 アドレスごとに割り振った番号というか、 一般に「IDナンバー」とか言われているものと同じ役目を持たせるつもりなのでは。 IDナンバーって何だと言われるとそれも困るのだが。 とりあえず、code name である必要はなくて、 この場合は1から10までの番号になるのだ。 これが0から9までならindexと呼べばすっきりするのだが。 もちろん、1から10がindexでも全然問題はないのだが、 Cの場合、言語的にはindexが0から始まるというような解釈の方が便利なので、 そこがひっかかるかもしれない。
ちなみに、IDナンバーは本当に必要なのか、という考え方もある。 個々の情報を識別できる情報があれば、連番を割り振る意味がないこともある。 例えば、データの作成時刻や、URLなどがデータに含まれている場合。 余談ですが。 で、本題に戻る。
int read_address(int fd, address_struct *p_address)
{
if (read(fd, &(p_address->code), sizeof(int)) < sizeof(int))
return -1;
if (read(fd, p_address->name, sizeof(p_address->name)) < sizeof(p_address->name))
return -1;
if (read(fd, p_address->add , sizeof(p_address->add)) < sizeof(p_address->add))
return -1;i
if (read(fd, p_address->tel, sizeof(p_address->tel)) < sizeof(p_address->tel))
return -1;
if (read(fd, p_address->mail, sizeof(p_address->mail)) < sizeof(p_address->mail))
return -1;
return 0;
}
何かいまいちというか、 こういう書き方をしたら仕様修正時にダブっている所を修正し忘れるという典型的な例みたいな気がしてきた。 ここを別関数にくくりだすリファクタリング。
int read_all(int fd, void *p, size_t size)
{
if (read(fd, p, size) < size)
return -1;
return 0;
}
int read_address(int fd, address_struct *p_address)
{
if (read_all(fd, &(p_address->code), sizeof(int)))
return -1;
if (read_all(fd, p_address->name, sizeof(p_address->name)))
return -1;
if (read_all(fd, p_address->address , sizeof(p_address->address)))
return -1;
if (read_all(fd, p_address->tel, sizeof(p_address->tel)))
return -1;
if (read_all(fd, p_address->mail, sizeof(p_address->mail)))
return -1;
return 0;
}
read_address、 何となく、次のように書いてみたくなるけど。
int read_address(int fd, address_struct *p_address)
{
if (read_all(fd, &(p_address->code), sizeof(int)) ||
read_all(fd, p_address->name, sizeof(p_address->name)) ||
read_all(fd, p_address->address , sizeof(p_address->address)) ||
read_all(fd, p_address->tel, sizeof(p_address->tel)) ||
read_all(fd, p_address->mail, sizeof(p_address->mail))) {
return -1;
}
return 0;
}
これはどちらでもいいと思う。 とりあえず、 これでコンパイル・ランさせてみると、 read error と表示された。 つまり、main の処理中で行われているはずの read のエラーチェックは、実は機能していなかったわけだ。
しかし、実は最初のコードのままで実行すると、 エラーも表示されないし、 画面に正しく構造体の中身が表示されているかのように見える。 これはある意味当たり前な話で、 read は 1バイトも読み込んでいないのだが、 サイズとして 0 を返すだけで別にエラーにはならないので、 エラーのチェックにはひっかからない。 だいたい、元のコードだと、 エラーが発生したとしても、 たまたま最後の処理でエラーが発生しなければ、 それ以前のエラーは全て無視して「エラーはありません」という判定になってしまうので、 ロジックがそもそもおかしい訳だが。
この read をテストのつもりで書いたのなら、 基本的な発想が一つおかしい。 テストに使う変数は使いまわしてはいけない。 write に使った変数をそのまま使ったために、 元の値がそっくり残っていて、 それが画面に表示されてしまったのでは。 address_test のような変数を新規に使ってテストすれば、 ここで既におかしいということが一発で分かったのだ。
で、先に進んで、loadですが。
char filename[20];
どういう処理系を使っているのか知らないが、 20は今時ないと思う。 って、実は CP/M を使っているとかいうのならごめんなさい。 Windows だとどうだっけ、 256まで使えた? 512とか1024という処理系もありそうだが、 とりあえず、このあたりはシステムがマクロで定数を定義していると思う。 FILENAME_MAX とか、 PATH_MAX のような定数がどこかに定義されているはずだ。
まあそれはいいとして、 20は20で構わない。 それよりも、 入力した結果が20文字未満であることをどこかで保証することの方が重要なのだ。 ちなみに、その重要さは、 256であろうが1024であろうが変わらないということに注意。 ファイル名の長さが256まで許されるのなら、257文字入る領域を確保しておいて、 なおかつ、実際に入力された名前が256文字以下であることをチェックするとか、 そういう処理が必要だ。
それはそうとして、 今回は作成されるファイル名が data1 に固定のようだし、 とりあえずそこは放置して先を見てみる。
fd = open("filename", O_RDONLY);
書き込んだ直後に意味のないテストをしようとしているのに、 どうしてこの重要な箇所にエラーチェックを入れようとしなかったのだろうか? 一つの原因は、処理が分散しているからといえるだろう。 「読み込むファイルを開く」という処理が一箇所になっていたら、 そこでチェックしていたらチェックは完璧なのである。 もっとも、そこでチェックを忘れたら全部忘れることになるのだが。
read(fd , pt , sizeof(address_struct)*10 - 1);
もちろん、これを見た瞬間に最大のバグに気づかなければならないが、 それは後回しにして、些細な謎が。 どうして3番目の引数、1を引いているのだろうか?
1引くというのも十分ヤバいのだが、 もう少し細かい話をしてみると、 このプログラム、 10個書き込んであればいいが、 必ずしも10個分のデータを書き込むとは限らないはず。 5個とか6個のデータしか書き込んでいない状態で、 10個まるごと読み込もうとするとどうなるか。 ここで、最初のコードをよく見てみると、 画面には「(終了時にはコード0)」と書いてあるが、 入力されたのが0かどうかというチェックはしていないのである。 というか、1〜10以外の数値なら何でも break しているので、 そう考えたら確かに 0 を入れたら終了するという動作にはなっているか。 しかし、 「１〜１０の数字をコードに入力してください」 と画面に表示されたら時すでに遅しで、 ループから出てしまうので、 もうその数字をコードとして指定することはできないのだ。
何か面倒になったので、すみませんが、ここで give up。残りは列挙。
・sys/types.h、sys/stat.h、string.h を include するのは意味がない。 もしかしたら処理系に依存するのか? こちらは cygwin + gcc でテストした。
・printf を使うのだったら stdio.h を include しなければならない。
・main() で定義されている変数 flg、filename は一度も使われていない。
・(多分これが一番大きなバグだが) メンバごとに write したのなら read(fd , pt , sizeof(add)*10 - 1); のように一度に読み込むことはできない。 たまたま読み込める場合もあるかもしれないが、それはたまたまである。
・load の処理内で、main で定義されている「テーブルのサイズは10である」という情報を使っている。 もし main 内の add addtable[10]; という所を add addtable[5]; とか add addtable[10000]; に変更したら、load も書き換えなければならないが、 書き換えなければならないという情報がどこにも現れていない。
・loadという関数の型はintだが、 何も値を返していないし、 呼び出した側も値を見ていない。
とりあえず中途半端に直してみたのがこれ。 一部の処理は、まだおかしいままだが。
#include <stdio.h>
#include <fcntl.h>
#include <unistd.h>
#include <ctype.h>
#include <stdlib.h>
/**
scanf は使うべきではないが、
修正するのが面倒なのでそのまま残してある。
*/
typedef struct {
int code;
char name[20];
char address[50];
char tel[15];
char mail[50];
} address_struct;
void write_address(int fd, address_struct address)
{
write(fd, &address.code, sizeof(int));
write(fd, address.name, sizeof(address.name));
write(fd, address.address , sizeof(address.address));
write(fd, address.tel, sizeof(address.tel));
write(fd, address.mail, sizeof(address.mail));
}
int read_all(int fd, void *p, size_t size)
{
if (read(fd, p, size) < size)
return -1;
return 0;
}
int read_address(int fd, address_struct *p_address)
{
if (read_all(fd, &(p_address->code), sizeof(int)))
return -1;
if (read_all(fd, p_address->name, sizeof(p_address->name)))
return -1;
if (read_all(fd, p_address->address , sizeof(p_address->address)))
return -1;
if (read_all(fd, p_address->tel, sizeof(p_address->tel)))
return -1;
if (read_all(fd, p_address->mail, sizeof(p_address->mail)))
return -1;
return 0;
}
void print_address(address_struct address)
{
printf("\n");
printf(" code : %d\n", address.code);
printf(" name : %s\n", address.name);
printf(" address : %s\n", address.address);
printf(" TEL.No : %s\n", address.tel);
printf(" mail.No : %s\n", address.mail);
}
int load(address_struct *table, size_t size){
int fd;
int i;
char filename[20];
printf("保存したファイルを開きます。\n");
printf("開きたいファイル名を指定してください\n");
scanf("%s" , filename); /* 入力文字列が20未満であるチェックが抜けている */
fd = open(filename, O_RDONLY);
if (fd == -1) {
fprintf(stderr, "open error! (%s)\n", filename);
return -1;
}
for (i = 0; i < size; i++) {
if (read_address(fd, table) < 0) {
fprintf(stderr, "read error!\n");
break;
}
print_address(*table);
table++;
}
close(fd);
return 0;
}
void create_data()
{
char filename[] = "data1";
address_struct a;
int fd;
int flgs =O_TRUNC | O_CREAT | O_RDWR;
int i;
int mode = 00744;
fd = open(filename, flgs, mode);
if (fd == -1) {
fprintf(stderr, "open error!\n");
exit(1);
}
printf("1〜10人までのコードを入力できます。(終了時はコード0）\n");
for(i = 0; i < 10; i++){
printf("\n code ?");
/* 終了時のコード0の判定が抜けている */
scanf("%d",&a.code);
if (isalpha(a.code) || a.code>10 || a.code<1) {
printf("１〜１０の数字をコードに入力してください\n");
break;
}
printf("Name ?");
scanf("%s",a.name);
printf("address?");
scanf("%s",a.address);
printf("TEL.No ?");
scanf("%s",a.tel);
printf("mail.No ?");
scanf("%s",a.mail);
write_address(fd, a);
/* ここでテストする意図が分からないので、とりあえずコメントにした。
* 書き込まれたことをテストしたいのなら、seek しなければならない。
* seek の後、次に書き込む位置を元に戻す処理も必要。
* (正常に読み込まれたら、次に書き込む位置になるか?)
*/
/*
{
address_struct address_test;
if (read_address(fd, &address_test) < 0) {
printf("read error\n");
exit(1);
}
print_address(address_test);
}
*/
}
close(fd);
}
void load_test()
{
address_struct address_table[10];
int bufnum;
printf("これからなにをしますか\n");
printf("1 : load , 2 : quit\n");
scanf("%d" ,&bufnum);
switch (bufnum) {
case 1 :
load(address_table, sizeof(address_table) / sizeof(address_struct));
break;
default:
break;
}
}
int main(){
create_data();
load_test();
return 0;
}
実は、編集もしたいというのだが、 このレベルのプログラミング技術で編集機能を作るというのはかなり無謀だ。 この調子で考えていると連載の原稿数回分のネタになりそうだが、 ちょっと時間もないし。 今回はここまで。
【回答】
read や write を使って構造体の内容を読み書きする場合は、 メンバ毎に情報を書き込んだら読み込む時もメンバ毎に処理し、 構造体全体をまるごと書き込んだら読み込む時もまるごと一度に処理する必要があります。
元のプログラムは、 構造体のメンバ単位に書き込む処理を行っていますが、 書き込んだ内容をまるごと読み込もうとしているので、 それがトラブルの原因になっている可能性があります。
但し、元のプログラムにはバグが多すぎるため、 実際に何がご指摘の「文字化け」の原因になっているか定かではありません。 失礼ながら、 プログラムを拝見した限りでは、 プログラミングの技術が低水準関数を扱うレベルに達していないと思われますので、 基本から復習してみた方がよいのではないかと思います。