c言語 セグメンテーション違反の修正

Question

###セグメンテーション違反を直したいです

c言語でスタックを作ったのですがpopを実行時にセグメンテーション違反が発生してしまいます。
popの関数ではスタックから値を一つ消したいのですが、スタックの中身がひとつもないときにpopを実行するとプログラムが終了するようにしたいです。
またpopによって中身が0個になった場合はそのことを表示したいです。
よろしくおねがいします。

該当のソースコード

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

void push( char );
char pop();
void free_stack();
void print_stack();
struct cell{
  char c;
  struct cell *next;
};
typedef struct cell cell;

cell *p;
cell *listhead;

//pushする関数
void push(char s/*pushする文字*/){
 p=(cell*)malloc(sizeof( cell ) );
    p->c = s;
    p->next = listhead;
    listhead = p;
}

//popする関数
char pop(){
  cell *tmp = NULL; 
  if(listhead == NULL ){
    printf("スタックは空です\n");
    free_stack();  
  }
  else{
    tmp=p->next;
    listhead = tmp;
    free(p);
  }
 print_stack();
      return tmp->c;
}

//end後スタックのメモリを開放する関数
void free_stack(){
  cell *q;
 while(p != NULL){
    q=p->next;
    free(p);
    p=q;
  }
 printf("プログラムを終了します\n");
}

//スタックの内容を表示する関数
void print_stack(){
  while(p != NULL){
 printf(" -> %c" , p->c);
      p=p->next;
} 
  printf("\n");
   
}

int main(void){
  char in[100];//スタック
  int n=0;//保存されているデータ数
  
  do{
    printf("pushする文字かpop/endを入力して下さい --> ");
    scanf("%s" , &in[n]); 
    //endの入力
    if(strcmp(&in[n],"end") == 0){
      printf("スタックを空にしました\n");
      free_stack();
}
    //popの処理
    else if (strcmp(&in[n],"pop") == 0){
      printf("popされた文字は %c です\n" , pop());
      if(n>0){
      n--;
      }
}
    //popやendでない時の処理
    else if(strlen(&in[n]) != 1){
      printf("不正な入力です\n");
      free_stack();
      break;
}
    //適切な入力の処理
    else{
      push(in[n]);
      print_stack();
      n++;
}

  }while(strcmp(&in[n],"end") != 0);

   
  return 0;
}

Comments

[itagagaki]

listheadがスタックの先頭を指しているっぽいですが、ではpは何を指していますか？

[thkana]

> よろしくおねがいします。
というのは、完成したソースを寄越せ、という意味ですか?

[tamintya]

pはグローバル宣言しているのでpush関数内での値などが入っているわけではないのですか?

[itagagaki]

tamintyaさんが、pという変数にどのような意味を持たせているのか、どのような値を保持し、いつどのように変更したり参照されるべきなのかを、どう設計し、どう把握しているのか、ということをお聞きしました。そこがかなり怪しいですね。

[tamintya]

リスト構造だからという感じでpを使っていたのでもう一度確認し直したいと思います。
ありがとうございました。

Answer 1

C言語のコードを書くなら、デバッグできる環境を整えましょう。
Eclipseや、WindowsならVisualStudioなど。
コードの任意の場所で実行を止め、変数のナカミを見ることができます。そこから１行づつ実行して、コードの流れを見れるようになります
そうすれば、アテズッポでコードを書かなくて済むようになります。

p に何も代入されてないように見えます。
なにがはいってるでしょうか

Comments

[tamintya]

回答ありがとうございます。
上にも書きましたがpはグローバル宣言しているのでpush関数内での値などが入っているわけではないのですか?

[y_waiwai]

pushが実行されたら、たしかにそこに値が入ります
逆に言うと、pushが実行されない限り、そこには値は入りません

で、どこでそれが実行されてますか？

[tamintya]

その場合はlisthead == NULL　となっているはずなのでpを使わないルートを用意しているはずなのですが?

[y_waiwai]

まあとにかく、デバッグ環境を整えて、変数のナカミを見れるようにしましょう。
XXのはず、XXと思うから、といってるうちは問題は解決しませんよ

[tamintya]

分かりました。
疑問点に答えて下さりありがとうございました。

Answer 2

セグメンテーションフォールトが起こるのは、pop() の中で tmp = p->next; をしてますが、これが原因。この時 p の値が NULL になっている。
なぜNULLになるかというと、main() 関数が push() した後 print_stack() しているから。
print_stack() は p が NULL になった時、終了してますよね。

アドバイス

1. pop の中で p->next から始めるのではなく、listhead を見ることから始めること。
2. cell を free した後にメンバー変数 c や next をアクセスしないよう、注意深くコーディングする
3. cell *p; をグローバル変数にするのではなく、各関数内のローカル変数にする

Comments

[tamintya]

アドバイスありがとうございます。
参考にして自分で作ったソースコードを修正したいと思います。

[asm]

main関数の変数inの扱いにもツッコミいれたい

[rubato6809]

そうそう、まず最初にヘンと思ったのは in[] の使い方でした。セグフォの原因じゃなかったので抜けてしまいました。

[rubato6809]

そこんとこ、epistemeさんのコードは普通の使い方のようなので、こちらはそのまま。

Answer 3

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

void push(char);
char pop();
void free_stack();
void print_stack();

struct cell {
    char c;
    struct cell* next;
};
typedef struct cell cell;

cell* listhead = NULL;

//pushする関数
void push(char s/*pushする文字*/) {
    cell* p = (cell*)malloc(sizeof(cell));
    p->c = s;
    p->next = listhead;
    listhead = p;
}

//popする関数
char pop() {
    char c = '\0';
    if (listhead == NULL) {
        printf("スタックは空です\n");
    } else {
        cell* tmp = listhead->next;
        c = listhead->c;
        free(listhead);
        listhead = tmp;
    }
    print_stack();
    return c;
}

//end後スタックのメモリを開放する関数
void free_stack() {
    cell* p = listhead;
    while (p != NULL) {
        cell* q = p->next;
        free(p);
        p = q;
    }
    printf("プログラムを終了します\n");
}

//スタックの内容を表示する関数
void print_stack() {
    cell* p = listhead;
    while (p != NULL) {
        printf(" -> %c", p->c);
        p = p->next;
    }
    printf("\n");
}

int main(void) {
    char line[64];
    do {
        printf("pushする文字かpop/endを入力して下さい --> ");
        scanf("%s", line);
        if (strcmp(line, "end") == 0) {
            printf("スタックを空にしました\n");
            free_stack();
        } else if (strcmp(line, "pop") == 0) {
            char c = pop();
            if ( c != '\0' ) {
                printf("popされた文字は %c です\n", c);
            }
        } else if (strlen(line) != 1) {
            printf("不正な入力です\n");
            free_stack();
            break;
        } else {
            push(line[0]);
            print_stack();
        }
    } while (strcmp(line, "end") != 0);
    return 0;
}

[おまけ] 僕ならこう書く

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

struct cell {
    char c;
    struct cell* next;
};

typedef struct stack_t {
  struct cell* head;
} stack;

void initialize(stack* stk) {
  stk->head = NULL;
}

//スタックの内容を表示する
void print_stack(const stack* stk) {
    struct cell* p = stk->head;
    while (p != NULL) {
        printf(" -> %c", p->c);
        p = p->next;
    }
    printf("\n");
}

//pushする
void push(stack* stk, char s) {
    struct cell* p = (struct cell*)malloc(sizeof(struct cell));
    p->c = s;
    p->next = stk->head;
    stk->head = p;
}

//popする(空のときは'\0'を返す)
char pop(stack* stk) {
    char c = '\0';
    if ( stk->head != NULL) {
        struct cell* tmp = stk->head->next;
        c = stk->head->c;
        free(stk->head);
        stk->head = tmp;
    }
    return c;
}

// スタックを空にする
void clear(stack* stk) {
    struct cell* p = stk->head;
    while (p != NULL) {
        struct cell* q = p->next;
        free(p);
        p = q;
    }
    stk->head = NULL;
}

int main(void) {
    stack stk;
    initialize(&stk);
    char line[64];
    do {
        printf("pushする文字かpop/endを入力して下さい --> ");
        scanf("%s", line);
        if (strcmp(line, "end") == 0) {
        } else if (strcmp(line, "pop") == 0) {
            char c = pop(&stk);
            print_stack(&stk);
            if ( c != '\0' ) {
                printf("popされた文字は %c です\n", c);
            } else {
                printf("スタックは空です\n");
            }
        } else if (strlen(line) != 1) {
            printf("不正な入力です\n");
            break;
        } else {
            push(&stk, line[0]);
            print_stack(&stk);
        }
    } while (strcmp(line, "end") != 0);
    clear(&stk);
    printf("スタックを空にしました\n");
    printf("プログラムを終了します\n");
    return 0;
}

Comments

[tamintya]

回答ありがとうございます。
上のソースコードを実行してみたのですがpop関数が2回走っている？　ようなのですが修正個所が分からないです。

[episteme]

あーごめんなさい。printf() 中の　pop() は c のマチガイ。(修正しておきました)
# いや修正箇所が分らんのはヤバいっしょ...

[tamintya]

ありがとうございました。