

以下のコードは、5つの数値を入力で受け取り、受け取った順にnumberlistとして出力しその後、逆順でnumberlistを出力するプログラムです。

NULLの扱いが悪いのか、双方向リスト自体が間違っているのかよく分かりません。

[code]

    #include <stdio.h>
    #include <stdlib.h>
    
    struct number{
      int num;
      struct number *next, *prev;
    };
    
    int main(){
      struct number *p, *head, *start, *last;
      int i;
      start=(struct number *)malloc(sizeof(struct number));
      last=(struct number *)malloc(sizeof(struct number));
      head=(struct number *)malloc(sizeof(struct number));
      head->next=head;
      head->prev=head;
      head->num=NULL;
      printf("input 5 number:");
      for(i=0;i<5;i++){
        if(i==0){
          p=(struct number *)malloc(sizeof(struct number));
          p->prev=head;
          start=p;
        }
        p->next=(struct number *)malloc(sizeof(struct number));
        (p->next)->prev=p;
        scanf("%d", &p->num);
        p=p->next;
        last=p;
      }
      last->next=head;
      p=start;
      printf("number list:");
      while(p->num!=NULL){
        if(p==last){
          printf("%d", p->num);
        }else{
        printf("%d, ", p->num);
        }
        p=p->next;
      }
      printf("\n");
      p=last;
      printf("number list:");
      while(p->num!=NULL){
        if(p==start){
          printf("%d", p->num);
        }else{
        printf("%d, ", p->num);
        }
        p=p->prev;
      }
      printf("\n");
      return 0;
    }
    
[/code]


CC BY-SA 4.0
2019-11-03T06:30:28.983
60211
2019-11-05T03:25:05.940
2019-11-05T03:25:05.940
26370
35091
question
c
双方向リストについて　構造体のポインタの扱い


**追記：元々の何が問題か？**  
大本の間違いは以下2点です。

  * リスト先頭/最終判定のために、無効なデータ(num=NULL)を持つダミーデータを付けようとして、最終ダミーデータの処理に失敗していること。
  * 有効な最後のデータから逆順表示しようとする際に、ダミーデータから始めていること。
[code]     先頭(ダミー)<=>データ１<=>...<=>データ５<=>最終(ダミー(になっていない))<=>先頭へ戻る

    
[/code]

カンマ問題は別にして、リスト処理を最小限の修正で済ませるには以下2点を行います。

  * `for`ループの`last=p;`の後に`last->num = NULL;`を追加する。
  * 逆順表示の開始ポインタ設定`p=last;`を`p=last->prev;`に変更する。

おそらくリストの終端を判定する方法を模索してダミーデータに行きつき、そのための処理を作ろうとして色々と冗長かつおかしな処理になったものと思われます。  
一応 **循環リスト** を意識して作られてはいるようです。  
**改訂：簡潔に作り変えるとすると、こんな風に考えられます。**

  * 変数は初期化しておきましょう。あと`head`は不要です。`last`もこの時点では不要です。
[code]     struct number *p, *head, *start, *last;

    
[/code]

  * これは不要です。
[code]     start=(struct number *)malloc(sizeof(struct number));

    last=(struct number *)malloc(sizeof(struct number));
    head=(struct number *)malloc(sizeof(struct number));
    head->next=head;
    head->prev=head;
    head->num=NULL;
    
[/code]

  * `for`ループの中は、何の処理・設定が必要で、その順序はどうやれば早くて少なくて済むか考えた方が良いでしょう。片方向リストの質問では指摘しませんでしたが、ループカウンタの`i`も`for`内でしか使わないので短くできます。  
細かいところはソースで。

  * これも不要です。
[code]     last->next=head;

    
[/code]

  * `while(p->num!=NULL){`の`num` が `NULL`(=0) でない間ループという判定は、ダミーデータが前提なので、全部有効なデータだとしてポインタが元に戻った判定に変更します。

  * 表示は、片方向リスト時の @metropolis さんの処理が簡潔ですね。

まとめると以下のようになります。

[code]

    int main() {
        struct number* p = NULL, * start = NULL;  // 初期化を必要なだけ行う
        printf("input 5 number:");
        for (int i = 0; i < 5; i++) {
            // 変数宣言およびループ内処理を整理
            p = (struct number*)malloc(sizeof(struct number));
            if (i == 0) {
                start = p;
                // 循環リストで最初は自分自身を指している
                p->prev = p;
                p->next = p;
            }
            else {
                // 循環リストの最後に挿入する処理
                p->prev = start->prev;
                p->next = start;
                start->prev = p;
                p->prev->next = p;
            }
            int result = scanf_s("%d", &p->num);  // 古いscanfの代り
        }
        // --以後は入力されたデータの表示処理--
        p = start;
        printf("number list:");
        do {
            printf("%d%s", p->num, (p->next != start ? ", " : ""));
            p = p->next;
        } while (p != start);
        printf("\n");
        struct number* last = start->prev; // 逆順表示の終了判定簡略化のため
        p = last;
        printf("number list:");
        do {
            printf("%d%s", p->num, (p->prev != last ? ", " : ""));
            p = p->prev;
        } while (p != last);
        printf("\n");
        return 0;
    }
    
[/code]

**こんな記事を参照してみてください。**  
[連結リスト①（単方向・線形） | Programming Place Plus アルゴリズムとデータ構造編【データ構造】
第３章](https://programming-
place.net/ppp/contents/algorithm/data_struct/003.html)  
[連結リスト②（双方向・循環） | Programming Place Plus アルゴリズムとデータ構造編【データ構造】
第４章](https://programming-
place.net/ppp/contents/algorithm/data_struct/004.html)


CC BY-SA 4.0
2019-11-03T08:16:31.417
60214
2019-11-04T02:45:36.600
2019-11-04T02:45:36.600
26370
26370
60211
answer


[kunif](https://ja.stackoverflow.com/users/26370/kunif)さんの[回答](https://ja.stackoverflow.com/a/60214/25936)と[片方向リストにおいて](https://ja.stackoverflow.com/q/60210/25936)という質問の[akira
ejiri](https://ja.stackoverflow.com/users/35558/akira-
ejiri)さんの[回答](https://ja.stackoverflow.com/a/60222/25936)を踏まえた回答です。

[code]

    #include <stdio.h>
    #include <stdlib.h>
    
    // typedefすると経済的だと思います
    // まとめて宣言するべきではありません
    /*
       struct number{
       int num;
       struct number *next, *prev;
       };
    */
    typedef struct tNumber{
        struct tNumber *prev;
        int num;
        struct tNumber *next;
    } TNumber;
    
    // mainの引数が必要ないならvoidにしましょう
    int main (void) {
        // マジックナンバーには名前をつけましょう
        enum { MaxDataQty=5 };
    
        // まとめて宣言するべきではありません
        //struct number *p, *head, *start, *last;
    
        // メモリは1回で必要なだけ確保するほうが経済的なようです
        // このまま進めますが、変数の名前は意味が分かりやすいものにするべきです
        //TNumber *p = (TNumber *)malloc(sizeof(TNumber));
        TNumber *p = (TNumber *)malloc(sizeof(TNumber) * MaxDataQty);
    
        // 宣言と初期化は同時に出来ます
        // 最終的に利用しなくなりました
        //TNumber *head = (TNumber *)malloc(sizeof(TNumber));
    
        // 最初のfor文内にある最初のif文の中身は初回のみ実行され、実質的に宣言の初期化です
        // 最終的にheadは利用しなくなりました
        //p->prev = head;
    
        // 宣言と初期化は同時に出来ます
        // 利用される前にpが代入されます
        //TNumber *start = (TNumber *)malloc(sizeof(TNumber)); 
        TNumber *start = p;
    
        // 宣言と初期化は同時に出来ます
        // 利用される前にpが代入されます
        // startの対義語はendだと思います
        //TNumber *last = (TNumber *)malloc(sizeof(TNumber));
        TNumber *end = p;
    
        // for文内でしか利用しないなら後から宣言/初期化出来ます
        //int i;
    
        // 上記のように宣言と初期化は同時に出来ます
        // 利用される前にpが代入されます
        //start = (TNumber *)malloc(sizeof(TNumber));
    
        // 上記のように宣言と初期化は同時に出来ます
        //last = (TNumber *)malloc(sizeof(TNumber));
    
        // 上記のように宣言と初期化は同時に出来ます
        //head = (TNumber *)malloc(sizeof(TNumber));
    
        // 最終的に利用しなくなりました
        //head->next = head;
        //head->prev = head;
    
        // head->numはint型ですがNULLはvoid型へのポインタなので警告されます
        //head->num = NULL;
    
        printf("input 5 number:");
    
        // ループカウンタはiでも構いませんが、idxでも長くはないと思います
        // マジックナンバーには名前をつけましょう
        //for(i=0;i<5;i++){
        for (size_t idx = 0; MaxDataQty > idx ; ++idx) {
            // このif文の中身は初回のみ実行され、実質的に宣言の初期化です
            /*
            if (idx == 0) {
                p=(TNumber *)malloc(sizeof(TNumber));
                p->prev=head;
                start=p;
            }
            */
    
            // p[idx]が現在のアドレスです
            // 次の分のメモリは確保してあります
            // forループ後のlast->next = head;から循環型リストを作成します
            //p->next = (TNumber *)malloc(sizeof(TNumber));
            p[idx].next = ((MaxDataQty - 1) > idx) ? &p[idx+1] : start;
    
            // p[idx]が現在のアドレスです
            //(p->next)->prev = p;
            (p[idx].next)->prev = &p[idx];
    
            // p[idx]が現在のアドレスです
            // 処理の内容を鑑みるとfor文の頭に持っていっても良いと思います
            //scanf("%d", &p->num);
            scanf("%d", &p[idx].num);
    
            // p[idx]が現在のアドレスなので必要なくなりました
            //p = p->next;
    
            // p[idx]が現在のアドレスです
            // startの対義語はendだと思います
            //last = p;
            end = &p[idx];
        }
    
        // おそらくはリストを循環型にしたいのではないかと判断できます
        // startの対義語はendだと思います
        // end->nextはstartになっているので必要無さそうです
        //last->next = head;
    
        p = start;
    
        // 2回使う文字列に名前を付けました
        const char cHEADER[] = "number list:";
        const char cBODY[] = "% d%s";
        const char cSEP[] = ",";
        const char cFOOTER[] = "\n";
    
        // 2回使う文字列に名前を付けました
        //printf("number list:");
        printf(cHEADER);
    
        // p->numはint型ですがNULLはvoid型へのポインタなので警告されます
        // [kunif](https://ja.stackoverflow.com/users/26370/kunif)さんの[回答](https://ja.stackoverflow.com/a/60214/25936)にあるようにdo-while文を利用します
        //while( p->num != NULL) {
        do {
            // "% d"とすると数字の前に半角スペースが1個入ります
            // printf内で3項演算子を使うとループ終了後の"\n"もまとめて書けるようになります
            /*
            if (p == last) {
                printf("%d", p->num);
            } else {
                printf("%d, ", p->num);
            }
            */
            // 2回使う文字列に名前を付けました
            // startの対義語はendだと思います
            printf(cBODY, p->num, (p != end) ? cSEP : cFOOTER);
    
            p = p->next;
        } while (start != p);
    
        // ループ内のprintfで書いたので必要なくなりました
        //printf("\n");
    
        // startの対義語はendだと思います
        //p = last;
        p = end;
    
        // 2回使う文字列に名前を付けました
        //printf("number list:");
        printf(cHEADER);
    
        // p->numはint型ですがNULLはvoid型へのポインタなので警告されます
        // [kunif](https://ja.stackoverflow.com/users/26370/kunif)さんの[回答](https://ja.stackoverflow.com/a/60214/25936)にあるようにdo-while文を利用します
        //while (p->num != NULL) {
        do {
            // "% d"とすると数字の前に半角スペースが1個入ります
            // printf内で3項演算子を使うとループ終了後の"\n"もまとめて書けるようになります
            /*
            if(p == start){
                printf("%d", p->num);
            } else {
                printf("%d, ", p->num);
            }
            */
            // 2回使う文字列に名前を付けました
            printf(cBODY, p->num, (p != start) ? cSEP : cFOOTER);
    
            p = p->prev;
        } while (end != p); // startの対義語はendだと思います
    
        // ループ内のprintfで書いたので必要なくなりました
        //printf("\n");
    
        // EXIT_SUCCESSはstdlib.hにあります
        return EXIT_SUCCESS;
    }
    
[/code]


CC BY-SA 4.0
2019-11-03T10:24:06.550
60218
2019-11-04T08:58:19.873
2019-11-04T08:58:19.873
25936
25936
60211
answer