コンパイルには成功したが実行出来ない

Question

前提・実現したいこと

スタック機能を実装し{}の整合性を判定する。

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

コンパイルは通ったが実行しようとすると何もメッセージが出ず終了する。

該当のソースコード

//スタック機能を用いた整合性判定プログラム
//左括弧ならスタックにpush　右括弧ならpopし同種類の括弧か調べる
//今回のデータのマックスは20にする　 
#include <stdio.h>
#include <stdlib.h>
#include <string.h>

typedef struct node{
    char data;
    struct node *next;
} Node;

typedef struct {
    Node *head;
    Node *crnt;
} List;

typedef struct {
    int max;
    int num;
    List stk;
} Stack;

static Node *AllocNode(void){
    return (Node *)calloc(1, sizeof(Node));
}

int push(Stack *s, char x){
    if(s -> num >= s -> max){
        return -1;
    }

    if(s -> num == 0){
        s -> stk.head = AllocNode();//お盆の一番上がhead    
    }
    else{
        s -> stk.crnt = AllocNode();
        s -> stk.crnt -> next = s -> stk.head;//headを移動
        s -> stk.head = s -> stk.crnt; 
    }
    s -> stk.head -> data = x;
    s -> num++;
    return 0;
}

int pop(Stack *s, char *x){
    Node *node;

    if(s -> num == 0){
        return -1;    
    }

    node = s -> stk.head;

    while(node != NULL){
        if(x == "]"){
            if(&node -> data == "["){
                s -> stk.crnt -> next = node -> next;                
            }
        }
        if(x == "}"){
            if(&node -> data == "{"){
                s -> stk.crnt -> next = node -> next;
            }
        }
        if(x == ")"){
            if(&node -> data == "("){
                s -> stk.crnt -> next = node -> next;
            }
        }
        s -> stk.crnt = node;
        node = node -> next;
    }
    
    *x = s -> stk.crnt -> data;

    s -> num--;
    free(node);
    return 0;
}

void display(Stack *s){
    Node *node;
    node = s -> stk.head;

    while(node != NULL){
        printf("%c\n", node -> data);
        node = node -> next;
    }
}
int main(){
    Stack *s;
    int i;
    int result = 0;
    char data[50];
    s -> max = 20;
    s -> num = 0;
    
    printf("data?\n");
    scanf("%s",data);

    for(i = 0; data[i] != NULL; ++i){
/////////////////////////1//////////////////////////////////
        if(&data[i] == "{" || &data[i] == "[" || &data[i] == "("){//push
            result = push(s, data[i]);
        }
/////////////////////////2//////////////////////////////////
        if(&data[i] == "}" || &data[i] == "]" || &data[i] == ")"){//pop
            result = pop(s, &data[i]);
        }
        
        
    }

    if(s -> stk.head != NULL || result == -1){
        printf("不整合\n");
    }
    else{
        printf("整合\n");
    }
    free(s);
    return 0;
}
    

試したこと

1と2どちらかコメントアウトすると実効できるが両方コメントアウトしないと失敗する。

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

bcc32

Answer 1

コンパイルには成功したが実行出来ない

コンパイルできたとは、要するに文法的誤りが無いというだけです。正しく動作するなんて保障はどこにもありません。プログラム開発に欠かせないデバッグは、コンパイルが通ってからが本番です。考えを改めて下さい笑。
以下、バグと思われる5点に気づきました。さらにリスト構造は改良できます。

似たようなのを先日実装（略）うまくいったのはなぜでしょうか

運がよかったからです。

int main(){
    Stack *s;       // 未初期化変数（値は不定）

    s -> max = 20;  // バグ
    s -> num = 0;   // バグ

    free(s);        // これもバグ

このコードはバグです。コンパイルが通って動作したとしてもバグはバグです。

（既に指摘されていますが）ポインタ変数 s はなにを指している（つもり）でしょうか。maxやnumは、いつ、どこで、どうやってメモリが割り当てられたでしょうか。

ポインタ変数は、アドレスを値とする変数です。 Stack *s; はポインタ用のメモリを割り当てただけです。ポインタ変数 s の値は不定なので、どこのメモリを指しているか不明です。maxもnumも、正当に割り当てられていない、ということ。
同様のバグを書く初心者には「ポインタ変数を宣言しただけで、ポインタが指す先のメモリも割り当てられる」と誤解してる人がいるそうですが、誤りです。

次のコードもバグです（既に指摘されています）。

    char data[50];

    for(i = 0; data[i] != NULL; ++i){
        if(&data[i] == "{" ||

pop()関数の中でループするのもバグです。たぶん勘違いしていると思います。
（実現方法はいろいろあるとしても）普通 pop() はスタックの先頭からひとつだけ取り出すのが役目であり、ループする必要はありません。取り出したモノが、右カッコに対応した左カッコかどうかを調べるのは pop() を呼び出した側で行うべきことです。

main()関数の中の for ループの中で push(), pop() を呼び、成功したか否かをresult変数に代入していますが、その判定が for ループを抜けた後・・・これでは判定が遅すぎます。これもバグです。

普通スタックは配列で作りますが、それを単方向リストで作っていますね。
リスト構造はデータひとつずつにポインタ（struct node *next）が要るので、その分メモリが余計に必要なのは欠点ですが、リスト構造ならいくつでも積める柔軟さがあると思いました。
配列は、配列の大きさを（20個なら20個と）最初から決めておくのが基本ですが、リスト構造は上限を決めておく必要がありません。つまり int max は不要にできる。

そのデータ構造を Stack, List, Node と三段階にしているのは、改良の余地があります。List の中の Node *head ポインタは Stack 構造体の中に持たせれば良いことで、まずは Stack と Node の二段階にしたらどうでしょうか。Node *crnt も不要です（もっと言えば int num も Stack 構造体も不要で、Node *head ポインタさえあれば十分ですが）。

Stack 構造体を残した形で書くと、push(), pop() の肝心部分は次のようになります。num が０か否かの判定は不要です。

typedef struct node {
    char data;
    struct node *next;
} Node;

typedef struct {
    int num;
    Node *head;
} Stack;

int push(Stack *s, char x)
{
    Node *node = AllocNode();  // 新しいNodeを割当る

    node->data = x;
    node->next = s->head;      // 今までのリストを次につなげ、
    s->head = node;            // 新しいNodeを先頭にする
    ...

int pop(Stack *s, char *x)
{
    Node *node = s->head;      // スタックトップを

    s->head = node->next;      // リストから外す
    *x = node->data;
    ...

上記のコードにするなら、最初に s->head = NULL; と初期化しておくと、リストの最後（最初にpushしたNode）の next ポインタには NULL が代入され都合が良いのですが、質問者のコードにはこれに対応するコードが見当たりません。それなのに display() 関数はリストの最後の NULL を期待しています。従ってこれもバグです。

以上、リスト構造の扱いはこれから使い慣れていけばよいことで、後回しで構わないと思いますが、Stack *s; s->max = 20; と if(&data[i] == "{" の件は至急見直すべき重大なバグです。

Answer 2

p->max=20の時点でメモリアクセス違反ですね。pは先がありません。

Answer 3

&data[i] == "{"では（アドレスが違うので）等しくなりません。data[i] == '{'ではないですか？
他の部分も同じです。文字列("{"など)のアドレスと配列(data)のアドレスが等しくなることはありません。
「追記」

#include <stdio.h>

int main(int agc, char *agv[])
{
    if( agc != 2 ){
        return 1;
    }
    //
    char paren[10];
    char *ap= agv[1];
    size_t  pos=0;
    //
    while(*ap){
        char ch= *ap++;
        if(ch == '[' || ch == '{' || ch == '('){
            paren[pos++] = ch;
        }
        if(ch == ']' || ch == '}' || ch == ')'){
            char c = '(';
            switch(ch){
            case ']': c= '[';
                break;
            case '}': c= '{';
                break;
            }
            //
            if(0 < pos  && paren[pos-1] == c){
                --pos;
            }else{
                return (int)pos;
            }
        }
    }
    //
    return 0;
}
 

Comments

[hama1185]

回答ありがとうございます
&を消しても動作は変わらず
警告文で移植性のないポインタ変換といわれたので&を付けました

[cateye]

それは、"{"が文字列(アドレス)と解釈されるからです。'{'(文字)にしましょうｄ＾＾

[hama1185]

警告吐かれなくなりました　ありがとうございます！

[cateye]

他のコメントにも書きましたが、カッコの数が合えばいいわけではないですよね。([{])} と入力があった場合、数は合いますがおかしいですよね？ 見間違いならいいのですが・・・そこはどうでしょう？

[hama1185]

考慮して作りなおしてます
似たようなのを先日実装してるのですがstackを実装しなくてもうまくいったのはなぜでしょうか
//スタック機能を用いた整合性判定プログラム
//左括弧ならスタックにpush　右括弧ならpopし同種類の括弧か調べる
//今回のデータのマックスは20にする　
#include <stdio.h>
#include <stdlib.h>

typedef struct node{
char data;
struct node *next;
} Node;

typedef struct {
Node *head;
Node *crnt;
} List;

typedef struct {
int max;
int num;
List stk;
} Stack;

static Node *AllocNode(void){
return (Node *)calloc(1, sizeof(Node));
}

int push(Stack *s, char x){
if(s -> num >= s -> max){
return -1;
}

if(s -> num == 0){
s -> stk.head = AllocNode();//お盆の一番上がhead
}
else{
s -> stk.crnt = AllocNode();
s -> stk.crnt -> next = s -> stk.head;//headを移動
s -> stk.head = s -> stk.crnt;
}
s -> stk.head -> data = x;
s -> num++;
return 0;
}

int pop(Stack *s, char *x){
Node *node;
int i;

if(s -> num == 0){
return -1;
}
node = s -> stk.head;
*x = s -> stk.head -> data;
s -> stk.head = s -> stk.head -> next;
s -> num--;
free(node);
return 0;
}

void display(Stack *s){
Node *node;
node = s -> stk.head;

while(node != NULL){
printf("%c\n", node -> data);
node = node -> next;
}
}
int main(){
Stack *s;
int memu;
char data;
char dummy;//ダミー
s -> max = 20;
s -> num = 0;

printf("_____________Menu_____________\n");
printf("______________________________\n");
printf("1->push 2->pop 3->show 0->exit\n");
printf("char型なので文字の入力\n");
while(1){
printf("number?\n");
scanf("%d", &memu);
if(memu == 0){
break;
}
switch(memu){
case 1://push
printf("data?\n");
scanf("%c",&dummy);
scanf("%c", &data);

if(push(s, data) == -1){
printf("failed\n");
}
break;
case 2://pop
if(pop(s, &data) == -1){
printf("failed\n");
}
else{
printf("%c\n", data);
}
break;
case 3://display
printf("________\n");
display(s);
break;
default:
break;
}
}
free(s);
return 0;
}

[cateye]

そろそろ出かけます。３連休は３出勤です；；
夜しか居れないので・・・
※文字列と文字の使い分けをしっかり！
ヒント：[{(が出たらスタックに積む、で]})が来たらそれに対応したカッコはスタックの先頭になくてはならない。在ったらpop、なかったらNG

[cateye]

usr~/test/c % ./a.out "[]{{[}}"
usr~/test/c % echo $?
3
usr~/test/c % ./a.out "[]{{[]}}"
usr~/test/c % echo $?
0
usr~/test/c %
ソースは追記してあります。（リンクリストではありません）・・・参考までに

[hama1185]

同じく4日間学会でこの時間帯しかいれないです...ちょっと実装遅れるのでご了承ください。

[cateye]

了解

[hama1185]

遅れてしまい申し訳ありません。一応実装し、予想通りの結果を得ることができました。ありがとうございました！

Answer 4

Stack *s;

初期化されてません。
これはなにを指している（つもり）でしょうか

Comments

[hama1185]

回答ありがとうございます
スタックなどをまとめた構造体のポインタのつもりです

[hama1185]

Stack をポインタにせず変数にするということでしょうか
'''C
//スタック機能を用いた整合性判定プログラム
//左括弧ならスタックにpush　右括弧ならpopし同種類の括弧か調べる
//今回のデータのマックスは20にする　
#include <stdio.h>
#include <stdlib.h>
#include <string.h>

typedef struct node{
char data;
struct node *next;
} Node;

typedef struct {
Node *head;
Node *crnt;
} List;

typedef struct {
int max;
int num;
List stk;
} Stack;

static Node *AllocNode(void){
return (Node *)calloc(1, sizeof(Node));
}

int push(Stack *s, char x){
if(s -> num >= s -> max){
return -1;
}

if(s -> num == 0){
s -> stk.head = AllocNode();//お盆の一番上がhead
}
else{
s -> stk.crnt = AllocNode();
s -> stk.crnt -> next = s -> stk.head;//headを移動
s -> stk.head = s -> stk.crnt;
}
s -> stk.head -> data = x;
s -> num++;
return 0;
}

int pop(Stack *s, char *x){
Node *node;

if(s -> num == 0){
return -1;
}

node = s -> stk.head;

while(node != NULL){
if(x == "]"){
if(&node -> data == "["){
s -> stk.crnt -> next = node -> next;
}
}
if(x == "}"){
if(&node -> data == "{"){
s -> stk.crnt -> next = node -> next;
}
}
if(x == ")"){
if(&node -> data == "("){
s -> stk.crnt -> next = node -> next;
}
}
s -> stk.crnt = node;
node = node -> next;
}

*x = s -> stk.crnt -> data;

s -> num--;
free(node);
return 0;
}

void display(Stack *s){
Node *node;
node = s -> stk.head;

while(node != NULL){
printf("%c\n", node -> data);
node = node -> next;
}
}
int main(){
Stack s;
int i;
int result = 0;
char data[50];
s.max = 20;
s.num = 0;

printf("data?\n");
scanf("%s",data);

for(i = 0; data[i] != NULL; ++i){

if(data[i] == "{" || data[i] == "[" || data[i] == "("){//push
result = push(&s, data[i]);
}

if(data[i] == "}" || data[i] == "]" || data[i] == ")"){//pop
result = pop(&s, &data[i]);
}

}

if(s.stk.head != NULL || result == -1){
printf("不整合\n");
}
else{
printf("整合\n");
}
//free(s);
return 0;
}

'''

[cateye]

Stackの実態がないということです。

[hama1185]

よくわからないので詳しく教えていただけますか？

[cateye]

ポインタを使うなら、Stack stack;とか実態を作って、Stack * s= &stack; とかすればいいと言うことです。

[hama1185]

今変数で書き換えていたのですがcateyeさんはポインタと変数どちらが実装しやすいですか？

[cateye]

その前に、if (x == "]") {　等の修正は出来ましたか？・・・これは、if (*x == ']') {と成るはずですが？
今のままではちゃんと動きませんよ。また、たとえば[[{{)}}]のようにカッコが記述された場合はどうなりますか？・・・プログラムに書いた通り処理を追いかけてみましょう。（机上デバッグ）
#しいて言えばポインター派かなぁｗ・・・