プログラムを実行するとSegmentation fault (コアダンプ)と表示されてしまう

Question

実現したいこと

二分探索木を用いた電話帳検索システムのデータの挿入を行う関数を作成したい。

前提

キーボードから名前と電話番号を入力して二分探索木を構築するプログラムを作成したのですが、実行すると最初の名前と電話番号の入力を行った後にSegmentation fault (コアダンプ)表示されてしまいます。原因と解決方法を教えてください。

発生している問題・エラーメッセージ

ノードを追加しますか？
[Yes] ==> 1   [No] ==> 0
1
名前を入力してください。
souseki_natsume
電話番号を入力してください。
019-887-8765
Segmentation fault (コアダンプ)

該当のソースコード

#include <stdio.h>
#include <string.h>
#include <stdlib.h>
#define NUMBER 10000

typedef struct node{
  char name[NUMBER];
  char cellnumb[NUMBER];
  struct node *left;
  struct node *right;
}Node;

Node *addData(Node *target_node, Node *add_node)
{
  int n;
  n = strcmp(target_node->name, add_node->name);
  if(target_node == NULL){
    Node *target_node = malloc(sizeof(Node));
    strcpy(target_node->name, add_node->name);
    strcpy(target_node->cellnumb, add_node->cellnumb);
    target_node->left = NULL;
    target_node->right = NULL;
    printf("%s", target_node->name);
  }
  else if(n > 0){
    target_node->left = addData(target_node->left, add_node);
  }
  else if(n < 0){
    target_node->right = addData(target_node->right, add_node);
  }
  else if(n = 0)
    printf("同姓同名の人が居ます。\n");
    return target_node;
}


int main(void)
{
  int answer;
  Node *root;
  Node *p;

  root = NULL;
  do{
    int answer;
    printf("ノードを追加しますか？\n[Yes] ==> 1   [No] ==> 0\n");  scanf("%d",&answer);
    if(answer == 1){
      printf("名前を入力してください。\n");  scanf("%s", p->name);
      printf("電話番号を入力してください。\n");  scanf("%s", p->cellnumb);
      p->left = NULL;
      p->right = NULL;
      root = addData(root, p);
    }
  }while(answer == 1);

  return 0;
}

修正後のソースコード

#include <stdio.h>
#include <string.h>
#include <stdlib.h>
#define NUMBER 10000

typedef struct node{
  char name[NUMBER];
  char cellnumb[NUMBER];
  struct node *left;
  struct node *right;
}Node;

Node *addData(Node *target_node, Node *add_node)
{
  if(target_node == NULL){
    target_node =(Node*)malloc(sizeof(Node));
    strcpy(target_node->name, add_node->name);
    strcpy(target_node->cellnumb, add_node->cellnumb);
    target_node->left = NULL;
    target_node->right = NULL;
    printf("%s", target_node->name);
  }
  else if(strcmp(target_node->name, add_node->name) > 0){
    target_node->left = addData(target_node->left, add_node);
  }
  else if(strcmp(target_node->name, add_node->name) < 0){
    target_node->right = addData(target_node->right, add_node);
  }
  else
    printf("同姓同名の人が居ます。\n");
    return target_node;
}


int main(void)
{
  int answer;
  Node *root;
  Node *p;

  root = NULL;
  do{
    printf("ノードを追加しますか？\n[Yes] ==> 1   [No] ==> 0\n");  scanf("%d",&answer);
    if(answer == 1){
      printf("名前を入力してください。\n");  scanf("%s", p->name);
      printf("電話番号を入力してください。\n");  scanf("%s", p->cellnumb);
      p->left = NULL;
      p->right = NULL;
      root = addData(root, p);
    }
  }while(answer == 1);

  return 0;
}

修正後（２）のソースコード

#include <stdio.h>
#include <string.h>
#include <stdlib.h>
#define NUMBER 10000

typedef struct node{
  char name[NUMBER];
  char cellnumb[NUMBER];
  struct node *left;
  struct node *right;
}Node;

Node *addData(Node *add_node, Node *target_node)
{
  if (target_node == NULL){
    target_node = malloc(sizeof(Node));
    memcpy(target_node, add_node, sizeof(Node));
  }
    else{
      int cmp = strcmp(target_node->name, add_node->name);
      if (cmp > 0)
        target_node->left = addData(add_node, target_node->left);
      else if (cmp < 0)
        target_node->right = addData(add_node, target_node->right);
      else
        puts("同姓同名の人が居ます。");
    }
  return target_node;
}

void showALLData(Node *node)
{
  if(node != NULL){
    showALLData(node->left);
    printf("name : %s       cellnumber : %s\n", node->name, node->cellnumb);
    showALLData(node->right);
  }
}

 void freeTree(Node *node) {
   if(node == NULL) return;
   freeTree(node->left);
   freeTree(node->right);
   free(node);
 }

int main(void)
{
  int answer;
  Node *root = NULL;

  do{
    printf("ノードを追加しますか？\n[Yes] ==> 1   [No] ==> 0\n");  scanf("%d",&answer);
    if(answer == 1){
      Node p = {0};
      printf("名前を入力してください。\n");  scanf("%s", p.name);
      printf("電話番号を入力してください。\n");  scanf("%s", p.cellnumb);
      root = addData(&p, root);
    }
    showALLData(root);
  }while(answer == 1);

  freeTree(root);

  return 0;
}

試したこと

ここに問題に対して試したことを記載してください。

補足情報（FW/ツールのバージョンなど）

ここにより詳細な情報を記載してください。

Answer 1

teratail はデバッグしてもらう所ではありません。
最初から、各変数の値がどう変化しながら動作していくのかをしっかり意識して追って見てください。

入力時、 p は不定です。
printf("名前を入力してください。\n"); scanf("%s", p->name);
printf("電話番号を入力してください。\n"); scanf("%s", p->cellnumb);
p->left = NULL;
p->right = NULL;
の各 p-> ～ は、何処にアクセスするか分かりません。

addData の最初の呼び出しは、 root は NULL です。
root → target_node で
n = strcmp(target_node->name, add_node->name) を行おうとすれば、 target_node->name はアクセスできません。

ついでに、 main の do-while ループの条件に使っている answer と、追加するかの入力で使っている answer は、スコープが違う別変数です。

---

malloc を使うなら、同時に free も作っておいたほうが良いです。
結果確認用の表示も必要でしょう。

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

#define BUFSIZE 1024

typedef struct node {
  char name[BUFSIZE];
  char cellnumb[BUFSIZE];
  struct node *left, *right;
} Node;

Node *addData(Node *target_node, Node *add_node) {
  if(target_node == NULL) {
    target_node = malloc(sizeof(Node));
    memcpy(target_node, add_node, sizeof(Node));
    //printf("%s\n", target_node->name);
  } else {
    int cmp = strcmp(target_node->name, add_node->name);
    if(cmp == 0) {
      printf("同姓同名の人が居ます。\n");
    } else {
      Node **p = cmp > 0 ? &target_node->left : &target_node->right;
      *p = addData(*p, add_node);
    }
  }
  return target_node;
}

void printTree(Node *node, int indent) {
  if(node == NULL) {
    for(int i=0; i<indent; i++) printf(" ");
    if(indent > 0) printf("+");
    printf("(null)\n");
  } else {
    printTree(node->left, indent + 2);
    for(int i=0; i<indent; i++) printf(" ");
    if(indent > 0) printf("+");
    printf("%s\n", node->name);
    printTree(node->right, indent + 2);
  }
}

void freeTree(Node *node) {
  if(node == NULL) return;
  freeTree(node->left);
  freeTree(node->right);
  free(node);
}

Node *inputData(Node *root) {
  Node w = {0};
  printf("名前を入力してください。\n");
  scanf("%s", w.name);
  printf("電話番号を入力してください。\n");
  scanf("%s\n", w.cellnumb);
  return addData(root, &w);
}

int confirm() {
  int answer;
  printf("ノードを追加しますか？\n[Yes] ==> 1   [No] ==> 0\n");
  scanf("%d", &answer);
  return answer == 1;
}

int main(void) {
  Node *root = NULL;
  while(confirm()) root = inputData(root);
  printTree(root, 0);
  freeTree(root);
  return 0;
}

Comments

[KotatsuNeko]

回答ありがとうございます。今後は具体的な質問ができるようにします。回答を参考に少し修正しました。

[jimbe]

p が不定なのは修正されていません。
scanf が動いているから大丈夫…な訳ではありません。
運が良ければ動き悪ければ異常終了するという、非常に面倒なバグの元です。

またついでですが、 addData の最後の return はインデントが間違っています。

[KotatsuNeko]

丁寧な回答ありがとうございました。回答を参考に修正したところ意図した通りに動作しました。大変助かりました。

Answer 2

18行目で、仮引数target_nodeと同名の別の変数target_nodeを宣言してますが、
この変数は18～23行目の範囲だけで有効です。そこに勘違いがあるのでは？
他にもっと間違いがあるのかも知れませんが。

Comments

[otn]

あと、
> else if(n = 0)
比較なら = じゃなくて == です。= は代入。

[KotatsuNeko]

回答ありがとうございます。少し修正しました。

[otn]

ロジックを追ってみているわけでは無いですが、一見してわかる明らかな間違いはなさそうです。
あとは、1つずつ処理を確認しながらロジックを追うのでしょうか。もしくはデバッガーを使うか。
（このサイズのプログラムなら、机上でロジックを追う方が力が付くと思いますが）

前回・今回の質問のコードの間違いを見ても、記述の意味をちゃんと考えながら書いているようには思えないです。= と == はうっかりミスとしても。
プログラムは書いた通りにしか動かないので、しっかりと意味を考えながら書きましょう。

[KotatsuNeko]

ポインタの理解が追い付いていないので、じっくり考えながら勉強していこうと思います。ありがとうございました。

[otn]

自分で意味をきちんと理解していないコードを書かないと言うことを徹底すれば向上すると思います。
つまり、書こうとするコードを完全に理解できるまで学習するということです。入門書を復習しても良いし、理解するための短いプログラムを自分で書いてみて実行して、予想通りの結果に納得するという方法もあります。予想通りの結果で無い場合は理解のどこかが間違っていることになるので、細かく分解して1ステップずつ進み直します。

[KotatsuNeko]

入門書を復習していこうと思います。丁寧なアドバイスありがとうございました。