C言語 家系図

問題
構造体personを以下のように仮定する。

struct person {
int age;
char name[20];
struct person *father;
struct person *mother;
};

この構造体の表す人の名前、年齢、その人の父親の名前、およびその人の母親の名前を出力する関数

void print_person(struct person *p)

を作成せよ。出力の形式は

name: 本人の名前
age: 本人の年齢
father: 父親の名前
mother: 母親の名前

となるようにすること。
また、ポインタ father や mother の値が NULL のときには、名前のかわりに unknown と出力するようにせよ。


以上が問題なのですが自分でプログラムを作ってみたところ実行したら、エラーになって矯正終了されてしまいました。
以下が私の作ったプログラムです。

#include <stdio.h>

struct person {
int age;
char name[20];
struct person *father;
struct person *mother;
};

void set_name(struct person *p, char name[]) {
int i;
i = 0;
while (name[i] != 0) {
p->name[i] = name[i];
i++;
}
p->name[i] = 0;
}

void print_person(struct person *p) {
printf("name:%s", p->name);
printf("age:%s\n",p->age);
if(p->father != NULL){
printf("father:%s\n",p->father);
}
else{
printf("unknown");
}
if(p->mother != NULL){
printf("mother:%s\n",p->mother);
}
else{
printf("unknown");
}
}



int main(void) {
struct person me, dad, mom;

set_name(&me, "Michael");
me.age = 16;
me.father = &dad;
me.mother = &mom;

set_name(&dad, "David");
dad.age = 38;
dad.father = NULL;
dad.mother = NULL;

set_name(&mom, "Susan");
mom.age = 36;
mom.father = NULL;
mom.mother = NULL;

print_person(&me);
print_person(&dad);
print_person(&mom);

return (0);
}


どこが違うのか教えていただけないでしょうか？

No.6 ベストアンサー 回答者： hanabutako 回答日時： 2012/12/27 09:53

#1さんの指摘通りだと思います。

手元で試した感じでもint型の表示をするところで%dを使い、fatherやmotherを表示するところはp->father->nameやp->mother->nameとすることで出力の形式に近い表示が出ます。

具体的な変更点はこんな感じです。
@@ -19,15 +19,15 @@

void print_person(struct person *p) {
printf("name:%s", p->name);
-printf("age:%s\n",p->age);
+printf("age:%d\n",p->age);
if(p->father != NULL){
-printf("father:%s\n",p->father);
+printf("father:%s\n",p->father->name);
}
else{
printf("unknown");
}
if(p->mother != NULL){
-printf("mother:%s\n",p->mother);
+printf("mother:%s\n",p->mother->name);
}
else{
printf("unknown");

#2さんの指摘は半分以上は間違っているのでその部分を訂正します。
> まず、mainは一番上に記述するのが作法です。

そんな必要は全くありません。

> 構造体とポインタの扱いと構造体宣言と実態とローカル変数とグローバル変数とかがゴッチャになっています。

このプログラムにグローバル変数はひとつも出てきていません。
set_nameなど、ポインター渡しがちゃんとできていますし、me.father = &dad;などポインターを使いこなせているように見えます。
おそらく、#2さんはポインターを使いこなせていないのではないかと予想します。

> 宣言したばかりで値は不定のはず。ありえない。

アドレスの代入なので問題ありません。構造体の実体のアドレスはここを実行した時には既に決まっています。

> char型なのに数値で判定？？？？？

この指摘は正しいと思います。これが文字列であることを考えると0ではなくnull character('\0')と比較するのが正しいです。

しかし、たいていのC言語処理系ではこの問題が露見することはないと思います。
C99の仕様書のドラフトらしきもの http://www.open-std.org/jtc1/sc22/wg14/www/docs/ …
の5.2.1 Character setsによれば、null characterとはすべてのビットが0となった値となっています。
一方、数値の0もすべてのビットが0となった値で表現されるのが普通だと思います。
よって、0と書いてあっても'\0'と書いてあっても同じ機械語に翻訳されて実行されるでしょう。

ちなみにこの箇所は自分だったらstrlcpyの使用を真っ先に検討します。
あるいは、strncpyしてdst[sizeof(dst) - 1] = '\0'するか。

> って何故に数値の0を入れる？

これも上記と同じで文字列なので'\0'で閉じるのが正しいと思います。

> この宣言が回帰的な宣言になっている。自分自身の中で自分自身を参照していることになる。

これは全く問題ありません。
リスト構造や木構造を作る場合に普通に使われる書き方です。

同じくC99の仕様書のドラフトらしきものに 6.7.2.1 Structure and union specifiers の2節にこういう文が出てきます。
| (略) (hence,
| a structure shall not contain an instance of itself, but may contain a pointer to an instance
| of itself), (略)

構造体がその実態を持つことはできませんが、ポインタ変数として自分を参照することは問題ありません。

#3さんがstrcpy関数を使うよう薦めていますが、個人的にはstrlcpyかstrncpyのあとに'\0'をつけるほうが良いと思います。

#5さんの
> ここのループ終了条件に0を使っているので、配列の範囲を超えて強制終了、ということではないかと。

というのはかなり眉唾です。
Intel x86 や AMD64などの環境だと、先にも述べた通り、0と'\0'は機械語レベルでは同じ値になっていることが普通だと思います。よって、これによる無限ループというのはまずありません。

ただ、文字の比較ですから、'\0'と比較する習慣をつけたほうが良いとは思います。

> この場合、print_personと、set_nameをプロトタイプ宣言する必要があります。そうしないとコンパイル・リンクできません。

厳しくチェックするコンパイラーでもない限り、警告が出るだけでコンパイルは出来ると思います。おっしゃるとおり、この場合はプログラムの最初のほうで関数のシグニチャをプロトタイプ宣言しておいたほうが良いです。
なお、同一ファイル内にprint_personやset_nameがあるのでリンクには全く問題がありません。


あとは落穂拾い的に気づいたことを指摘します。
1. struct personの中でchar name[20]と固定値を書いていますが、#define NAME_LENGTH 20などして、char name[NAME_LENGTH]としたほうが良いと思います。将来、名前が長い人が出てきた時に20から30に変えようとするとマクロだと1箇所の変更で済みますが、固定値を書いていると1箇所ではすみません。(ファイルに書き込んだりネットワークで渡したりするところで、名前が20文字だと想定していると1箇所の変更では済まないですが...)
2. while (name[i] != 0)の箇所は先にも述べたようにstrlcpyの使用を検討してください。strlcpy(p->name, name, NAME_LENGTH);とするか、strncpyを使った上で、strncpy(p->name, name, NAME_LENGTH); p->name[NAME_LENGTH - 1] = '\0';とするかです。
(strncpyを使う理由、strncpyのあとにp->name[NAME_LENGTH - 1] = '\0';とする理由はhttp://linuxjm.sourceforge.jp/html/LDP_man-pages … の注意を見るとよいでしょう。
3. 細かいことですが、"name: %s\n"と\nをつけたほうが良いです。
4. 細かいことですが、unknownのところはfather: unknown\nとしたほうがずっと見やすく表示されます。
5. いっそのこと与えられたものがNULLだったらunknownへのポインタを返し、そうでない場合は名前へのポインタを返す関数を作っても良いかもしれません。
char*
get_name(struct person *p)
{
if(p != NULL) {
return p->name;
} else {
return "unknown";
}
}
そうすると、fatherを表示する部分はprintf("father:%s\n", get_name(p->father));となります。

以上。
頑張って。

この回答へのお礼

非常に解説がわかりやすかったです。
ありがとうございます。
実際にコンパイルして成功しました。
またよろしくお願いします。

お礼日時：2013/01/04 17:09

No.7 回答者： Tacosan 回答日時： 2012/12/27 13:38

本題からはずれてしまうけど 1点だけ:

C において '\0' は「値が 0 であるような int 型の定数」になります. 一方 0 は「値が 0 であるような int 型の定数」です. したがって, 見た目を除き '\0' と 0 とは全く同じです. その結果として, 例えば
int *p = '\0';
も全く合法だったりするんだけどさすがにこれはぼろくそに突っ込まれそう (「見た目」以外は問題ないんだけど).

あと落穂ひろいの重箱の隅を突っついてみると strlcpy は標準じゃないので strncpy だけを挙げた方が安全だろうし, get_name の返り値の型は const char * にしたかったりする....

No.5 回答者： hirotn 回答日時： 2012/12/27 03:19

申し訳ありません。

訂正させてください。

1) while (name[i] != 0) { 0 → '\0'
ここのループ終了条件に0を使っているので、配列の範囲を超えて強制終了、ということではないかと。
文字列の終端ナル文字は\0です。

---

>まず、mainは一番上に記述するのが作法です。
これは違います。
この場合、print_personと、set_nameをプロトタイプ宣言する必要があります。そうしないとコンパイル・リンクできません。

void set_name(struct person *p, char name[]);
void print_person(struct person *p);

>この宣言が回帰的な宣言になっている。
これも問題ありません。（理由はリスト構造を調べていただければ分かります。）

No.4 回答者： Tacosan 回答日時： 2012/12/27 01:12

試してないけど #1 で指摘されている通りだと思う. コンパイラによっては警告を出してくれるよ.

ちなみに #2 は 100 % 間違っているので無視して OK.

No.3 回答者： hirotn 回答日時： 2012/12/27 01:09

1) while (name[i] != 0) { ← \0

ここのループ終了条件に0を使っているので、配列の範囲を超えて強制終了、ということではないかと。
文字列の終端ナル文字は\0です。

あるいはwhile{}をやめ、string.hをincludeして、strcpy関数を使います。
詳細は調べてください。

2) printf("age:%s\n",p->age);

#1の回答者の方の仰るとおりです。

No.2 回答者： papapa0427 回答日時： 2012/12/27 00:57

まず、mainは一番上に記述するのが作法です。

間違いだらけでどこをどう指摘したらよいのか判りませんね。
構造体とポインタの扱いと構造体宣言と実態とローカル変数とグローバル変数とかがゴッチャになっています。

＞int main(void) {
＞struct person me, dad, mom;
＞
＞set_name(&me, "Michael");
＞me.age = 16;
＞me.father = &dad;　　　　←宣言したばかりで値は不定のはず。ありえない。
＞me.mother = &mom;　　　　←宣言したばかりで値は不定のはず。ありえない。

＞void set_name(struct person *p, char name[]) {
＞int i;
＞i = 0;
＞while (name[i] != 0) {　←char型なのに数値で判定？？？？？
＞p->name[i] = name[i];
＞i++;
＞}
＞p->name[i] = 0;　　　　　←って何故に数値の0を入れる？
＞}

そもそも
＞struct person {
＞int age;
＞char name[20];
＞struct person *father;
＞struct person *mother;
＞};
この宣言が回帰的な宣言になっている。自分自身の中で自分自身を参照していることになる。

ありえないプログラムですね。

No.1 回答者： honor 回答日時： 2012/12/27 00:49

intや構造体をを%sで表示したらまずいのでは