二回以上関数を使用するとデータが置き換わってしまいます。

Question

前提・実現したいこと

C言語でスタックを実現する関数を作っています
２回Push関数でスタック内にデータを入れて、Print関数でスタック内を見てみると1回目に入れたはずのデータも2回目に入れたデータ置き換わってしまいます。

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

なし

該当のソースコード(main関数)

#include <stdio.h>
#include "mystack.h"

int main(void)
{
	int menu;
	char *data[100];
	struct student *seito[100];
	char n[16];
	int m,e;
	int i=0;
	CharStack s;

	if (Initialize(&s, 64) == -1) {
		puts("スタックの生成に失敗しました。");
		return 1;
	}

	while (1) {
		char x[48],x1[16],x2[16],x3[16];
		printf("現在のデータ数：%d / %d\n", Size(&s), Capacity(&s));
		printf("(1)プッシュ  \n(2)ポップしてデータベースに格納　(3)データベースの値を表示　(4)スタック内表示　(0) 終了：");
		scanf("%d", &menu);

		if (menu == 0) break;

		switch (menu) {
		case 1: /*--- プッシュ ---*/
			printf("データ：");
			scanf("%s %s %s", x1,x2,x3);
			sprintf(x,"%s %s %s",x1,x2,x3);
			if (Push(&s, x) == -1)
				puts("\aエラ－：プッシュに失敗しました。");
			break;

		case 2: /*--- ポップしてデータベースに格納 ---*/
			if (Pop(&s, &data[i]) == -1)
				puts("\aエラ－：ポップに失敗しました。");
			else
				printf("ポップしたデータは%sです。\n", data[i]);
				//ポップしたデータを文字列と数値に変換
				sscanf(data[i],"%s%d%d",n,&m,&e);
				//構造体の配列に格納
				seito[i] = set_student(n,m,e);
				i++;
			break;
		/*---データベースの値を表示---*/
		case 3:
			print_all_student(seito,i);
			break;
		case 4:
			Print(&s);
		}
	}

	Terminate(&s);

	return 0;
}


### 使用したヘッダファイル内の関数
```C
#ifndef MYSTACK_H
#define MYSTACK_H

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

typedef struct
{
	int max;	//スタックのサイズ
	int ptr;	//スタックのポインタ
	char **stk;	//スタック(の先頭要素へのポインタ)
} CharStack;

struct student
{
        char name[12];
        int math;
        int eng;
};
/*--- スタックの初期化 ---*/
int Initialize(CharStack *s, int max)
{
	s->ptr = 0;
	if ((s->stk = calloc(max, sizeof(char *))) == NULL) {
		s->max = 0;								/* 配列の生成に失敗 */
		return -1;
	}
	s->max = max;
	return 0;
}
/*--- スタックの後始末 ---*/
void Terminate(CharStack *s)
{
	if (s->stk != NULL)
		free(s->stk);		/* 配列を破棄 */
	s->max = s->ptr = 0;
}
/*--サイズ確認--*/
int Size(const CharStack *s)
{
	return s->ptr;
}
/*--- スタックにデータをプッシュ ---*/
int Push(CharStack *s, char *x)
{
	if (s->ptr >= s->max)						/* スタックは満杯 */
		return -1;
	s->stk[s->ptr++] = x;
	return 0;
}

/*--- スタックからデータをポップ ---*/
int Pop(CharStack *s, char **x)
{
	if (s->ptr <= 0)							/* スタックは空 */
		return -1;
	*x = s->stk[--s->ptr];
	return 0;
}
/*--- スタックの容量 ---*/
int Capacity(const CharStack *s)
{
	return s->max;
}
/*--- 全データの表示 ---*/
void Print(const CharStack *s)
{
	int i;

	for (i = 0; i < s->ptr; i++)		/* 底→頂上に走査 */
		printf("%s", s->stk[i]);
	putchar('\n');
}

void print_all_student(struct student *seito[],int ninzu)
{
	int i;
	for(i=0;i<ninzu;i++)
	{
	printf("Name:%s\nMath:%d\nEng:%d\n",seito[i]->name,seito[i]->math,seito[i]->eng);
	}
}
//struct student構造体オブジェクトのメンバに値を格納する関数
struct student *set_student(char *n, int m, int e)
{
        struct student *ps = NULL;
        ps = (struct student*)malloc(1 * sizeof(struct student));
        if(ps == NULL)
        {
                puts("記憶域の確保に失敗しました");
                return NULL;    
        }
        strcpy(ps->name, n);
        ps->math = m;
        ps->eng = e;
        return ps;
}

#endif

試したこと

Pushを二回行い、スタック内を確認したら問題が発生したので、Pushの関数かmain関数の変数に問題があるのかもしれないということがわかった。

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

visual C++ 2010

Answer 1

Pushに渡しているxがmainのローカル変数とかではないですか？
stkは各要素をchar*で保存していてすべて同じ変数を示しているためxを書き換えれば要素の値も変わってしまいます。
stkのポインタのアドレスを見れば多分全部同じになっていると思います。

保存するときにmalloc & strcpy, またはstrdupでメモリを確保して文字列を別の領域にコピーしなければいけません。
いらなくなったとき(Popして値を使用し終わった後)のfreeもお忘れなく。

追記

現状のコードだとwhileで繰り返す度にxは存在が消えるので大変危険なコードです。
一番簡単なのは

Push(&s, strdup(x))

とすれば新しいバッファを確保して文字列をコピーして返してくれます。
Popで得たデータはスタックにはもう残っていませんから、帰ってきたものを使い終わったらfreeで開放します。

char* p;
if (Pop(&s, &p) == -1)
    puts("\aエラ－：ポップに失敗しました。");
else {  // <------------------------ ブロックで囲むこと
    printf("ポップしたデータは%sです。\n", p);
    //ポップしたデータを文字列と数値に変換
    sscanf(p,"%s%d%d",n,&m,&e);
    //構造体の配列に格納
    seito[i] = set_student(n,m,e);
    i++;
    free(p); // 必要なくなったので開放
}

あとTerminateでも開放が必要です。

for(int i=0; i<s->ptr; i++)
    free(s->stk[i]);        // 残っている分を全て開放
free(s->stk);

Comments

[wagon]

Pushに渡すxを具体的にはどのように変更を加えればよいのでしょうか？

[wagon]

読み込むxを1ループごとに別の場所に保存していき、それをPushの引数にとるというように変更すればよいのでしょうか

[wagon]

おかげで解決しました。ありがとうございました。

Answer 2

そのPush、Pop関数はint*を読み書きしてますが、

        sprintf(x,"%s %s %s",x1,x2,x3);
        if (Push(&s, x) == -1)

x の出どころが不明です。
こいつの本体はどこに存在してるんでしょうか。

Comments

[wagon]

main関数内でchar x[48]で定義しました。
ソースコードをすべて記載したのでそちらを確認していただけると助かります。

[y_waiwai]

push/pop でアドレスを操作してますが、そのアドレスの刺し先は保存されません
２回目のpushの際、xの内容は上書きされ、pushされるアドレスxの値も同一のものになります

[wagon]

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