コンパイルは通るのですがエラーが出て実行できない

Question

コンパイルは通るのですがエラーが出て実行できない。とりあえずわかる範囲でデバッグしてみました。
エラーの箇所は実行結果に示してあります。。どう直せばいいかがわかりません。

コード
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <assert.h>	

#define N 256
#define FILENAME  "address.csv"

struct address{
  char number[N];
  char name[N];		// 名前
  char address[N];	// 住所　
  char tel[N];		// 電話
  struct address *before; 	// ソートに必要である ,前のリスト
  struct address *next;		// 次のリスト
  
};

void list_add(struct address **ap,char *number, char *name, char *address, char *tel) 
{
  struct address *p;
  if ((p = malloc(sizeof(struct address))) != 0) {
    strcpy(p->number, number);	
    strcpy(p->name, name);		
    strcpy(p->address, address);
    strcpy(p->tel, tel);		
    
    p->next = *ap;		// *apの先頭アドレスをポインタpのp->nextに代入する。
    					// p->nextはstruct address *pの中で次のリストをしめしている。
						// ポインタpの次のリストに追加したいstruct address **apの先頭アドレスを代入する。
    					// *apはheadである。
   printf("p:%p\tnext3: %p\tbefore: %p \t*ap: %p\n", p, p->next, p->before,*ap);
    if (p->next != 0)	// 代入された*apの先頭アドレスが0でないならば
		printf("p:%p\tnext3: %p\tbefore: %p\n", p, p->next, p->before);
      	p->next->before = p;
      	printf("p:%p\tnext3: %p\tbefore: %p\n", p, p->next, p->before);
    
    p->before = 0;
	  // *apをheadにするために次のリストのp->beforeを0にしている。
	printf("p:%p\tnext3: %p\tbefore: %p\n", p, p->next, p->before);
    *ap = p;					// *apをheadにしている。
    printf("p:%p\tnext3: %p\tbefore: %p \t*ap: %p\n", p, p->next, p->before,*ap);
  }
}

void release(struct address **ap) 
{
  if (*ap != 0) {
    release(&((*ap)->next));
    free(*ap);
    *ap = 0;
  }
}

void chop(char *p) {	
						
  for (; *p; p++)
    ;
  p--;
  while (*p == '\r' || *p == '\n')
    *(p--) = 0;
}

int main(void) 
{
  struct address *ap;
  FILE* fp;
  static char buff[N],number[N], name[N], address[N], tel[N];
  char *token=",";
  
  ap = 0;
  if ((fp = fopen(FILENAME,"r")) != 0) {
     while(fgets(buff, N, fp) != 0){
       chop(buff);
  	   strcpy(number, strtok(buff, token));
       strcpy(name, strtok(NULL, token));
       strcpy(address, strtok(NULL, token));
       strcpy(tel, strtok(NULL, token));
      
       list_add(&ap,number,name, address, tel);
     }
     fclose(fp);
  }  
  release(&ap);
  return 0;
}


/* 実行結果
とりあえずわかる範囲でデバッグしてみました。
エラーの箇所は以下で起きています。どう直せばいいかがわかりません。
main () at list_add.c:75
75                 strcpy(number, strtok(buff, token));
(gdb)
76             strcpy(name, strtok(NULL, token));
(gdb)
77             strcpy(address, strtok(NULL, token));
(gdb)
78             strcpy(tel, strtok(NULL, token));
(gdb)

Program received signal SIGSEGV, Segmentation fault.
0x77915633 in strcat () from C:\WINDOWS\SysWOW64\msvcrt.dll
(gdb)
*/

Answer 1

void chop(char *p) {
  for (; *p; p++)
    ;
  p--;
  while (*p == '\r' || *p == '\n')
    *(p--) = 0;
}

これだと"\0"を渡すと領域外にアクセスする

Comments

[退会済みユーザー]

解決策はどうすればいいですか。
あるプログラムを探して実行したときは実行できたのに、
今そのプログラムを実行してもエラーがでてしまいます。
そのプログラムからlist_addだけを実行するために抜粋してつくっています。

[KSwordOfHaste]

asmさんの修正コードですが、前半でPは必ずNUL文字を指すので後半の意味がなくないですか？もっと平易に書けばいいような気がします。
void chop(char *p) {
int len = strlen(p);
while (len > 0 && (p[len-1] == '\r' || p[len-1] == '\n')) *--p = '\0';
}
上記ではlen>0で「文字列先頭より前の領域を決してアクセスしない」という考え方にしてます。論理ができたらその後でポインターアクセスの最適化をすればよいのではないでしょうか。p[len-1]みたいなところの最適化です。

[asm]

(ﾉ∀`)ミスってた
修正ありがとうございます。

[KSwordOfHaste]

ポインターを最適化したコードは読みにくいですよね・・・細心の注意をしていてもやはりバグをつくりこんでまいがちと思います。

Answer 2

原因をどなたかがコメントされるかも知れませんが・・・

自分は問題発生時の取り組み方をコメントしてみようと思います。（すみませんがコードはよく見てません）

SIGSEGVですので、「アクセスしてはいけないアドレスをさわってしまった」ということですね。

• 不当アドレスの入ったポインターpに対して*pをアクセスした
• 正しいアドレスを指定せねばならない関数へ正しくないアドレスを渡した

例えばNULLを指定するとNGな引数にNULLを渡したとか・・・

• 文字列の終端文字(NUL文字)がなくてstrcpyで触ってはいけない領域に触れてしまった

などなど・・・

様々な原因が考えられます。特定するときのコツは「自分が正しいことをしている確信を持つ」ことです。確信を持つには関数の仕様やCの言語仕様の曖昧な点を（リファレンスや教科書を見直して）確認することも大事と思います。

そういう観点でより具体的なデバッグ結果を提示するとよりよい質問になると思います。（残念ながらご質問の内容ではデバッグしたとは言い難い・・・ですw;）異常終了した行が特定できているならその関数を呼び出す際にどのような引数を指定したかも当然しらべるべきですから。

どう直せばいいかがわかりません。

直し方より原因を掴むのが先ですよ！

---

追記：（大した話ではないですが）
異常終了する付近を調べる際、仕様を勘違いしていたり動作自体を勘違いしていたりといったことはよくあるので、少なくとも後者はデバッグプリントしておくと気づける場合もあります。

strcpy(name, strtok(NULL, x));
// ここでstrcpyの第二引数にNULLを渡すと致命的
// ならばデバッグするときは・・・
char* tmp1 = strtok(NULL, x));
printf("tmp1 = %p\n", tmp1);
strcpy(name, tmp1);
// などとすれば、より状況がつかみやすくなる・・・

Comments

[KSwordOfHaste]

やはりコメントがついてましたｗ；

[退会済みユーザー]

ありがとうございます。デバッグも勉強中でまだよくわからないまま、勉強のためもかねてやっています。
なんとか関数を調べてみます。

[退会済みユーザー]

メインのプログラムが正常動作しなくなっているので、そちらが原因かもしれません。
質問をし直します。こんやは今からお仕事ですので、次回暇なときにやります。