【C言語】シェルでリダイレクトを実装したいがプロセスが無限に作成されるエラーが起きる

Question

前提・実現したいこと

自作シェルでリダイレクト(>>)を実装したいのですが
以下のコードを実行すると無限ループに陥ってしまいます。
このコードのどこが無限ループの原因となっているのか教えていただきたいです。

---

【追記１】
指摘を受けて：

getCommandの他、省略していた関数やデバッグ文を記述しました。

リダイレクトのみ機能していない状態だったので該当のコードのみ載せたのですが誤解を招いてしまいすみません。

以下のコードをコンパイルして動かすとパイプ、リダイレクトを含まないコマンド及びパイプコマンドは問題なく動くのですが、リダイレクトの場合にプロセスが無限に作られてしまいCTRL+C等でプログラムを終了できない状態になってしまいます。

---

【追記２】
239行目にexit(1)を追加した所正常に動くようになりました。なぜここにexit(1)がないと無限プロセス作成エラーになるのか教えていただける方がいたらお願いしたいです。

該当のソースコード

#include <stdio.h>    
#include <stdlib.h>   
#include <unistd.h> 
#include <string.h>   
#include <errno.h>    
#include <sys/wait.h> 
#include <sys/stat.h>
#include <sys/types.h>
#include <fcntl.h>   
#define MAX_ARG 50
#define READ    0
#define WRITE   1

/* @戻り値	0 => 普通のコマンド, 1 => パイプコマンド, 2 => リダイレクトコマンド */
void Type(char* command, int* type, int* splitPosition){
    int pos = 0;

    (*type) = 0;

    while(command[pos] != '\0'){

        if(command[pos] == '|'){
            (*type) = 1;
            (*splitPosition) = pos;
        }
        else if(command[pos] == '>' && command[pos+1] == '>'){
            (*type) = 2;
            (*splitPosition) = pos;
        } 

        pos++;
    }
}

int substr(char* S1, char* S2, int P, int L){
    if( P < 0 || L < 0 || L > strlen(S1) ){
        return -1;
    }

    for( S1 += P; *S1 != '\0' && L > 0; L--){
        *S2++ = *S1++;
    }

    *S2 = '\0';

    return 0;
}


void split(char* command, char* part1, char* part2, int* type){
    int splitPosition = 0;

    Type(command, type, &splitPosition);

    #ifndef DEBUG
    printf("splitPosition = %d\n", splitPosition);
    #endif
    
    if( (*type) != 0 ){ 
        int start = ( (*type) == 1 )? splitPosition + 1 : splitPosition + 2; /* 修正：>> or | の前後にスペース無しに対応 */
        int len   = strlen(command) - start;
        
        if( substr(command, part1, 0, splitPosition) < 0 ||
            substr(command, part2, start, len) < 0){
            perror("substr\n");
            exit(1);
        }
    } else {
        strcpy(part1, command);
    }

    #ifndef DEBUG
    printf("part1 = %s\n", part1);
    printf("part2 = %s\n", part2);
    #endif
}

void getCommand(char* command){

    scanf("%255[^\n]%*[^\n]", command);
    scanf("%*c");
    
    #ifndef DEBUG
    printf("command = %s\n", command);
    #endif
}


int to_token(char* command, char** tokens){
    int argc = 0;

    tokens[argc] = strtok(command, " ");

    while(argc < MAX_ARG){
        tokens[++argc] = strtok(NULL, " ");

        if(tokens[argc] == NULL){
            break;
        }
    }

    return argc;
}


int Parse(char* command, char** parsedCommand){
    int argc = to_token(command, parsedCommand);
    int ret;
    
    if(strcmp( parsedCommand[argc - 1], "&") == 0){
        parsedCommand[argc - 1] = NULL;
        ret = 1;
    } else {
        ret = 0;
    }

    return ret;
}


void RegCmd(char* part1){
    char* parsedCommand[20];
    int background; 
    pid_t pid;

    background = Parse(part1, parsedCommand);

    if( (pid = fork()) == 0){
        execvp(parsedCommand[0], parsedCommand);
        perror("execvp failed\n");
        exit(1);
    } else {
        if(!background){
            waitpid(pid, NULL, 0);
        }
    }
}

void PipeCmd(char* part1, char* part2){
    char* cmd_left[20];
    char* cmd_right[20];
    int fds[2];
    pid_t pid1, pid2;
    int background;

    if( pipe(fds) < 0){
        perror("pipe failed\n");
        exit(1);
    } 


    background = Parse(part1, cmd_left) || 
                 Parse(part2, cmd_right);
    
    if( (pid1 = fork()) < 0){
        perror("fork failed\n");
        exit(1);
    } 

    if( pid1 == 0 ){
        dup2(fds[READ], 0);
        close(fds[WRITE]);
        execvp(cmd_right[0], cmd_right);
        perror("execvp failed\n");
    }
    
    if( (pid2 = fork()) < 0){
        perror("fork failed\n");
        exit(1);
    }

    if( pid2 == 0 ){
        dup2(fds[WRITE], 1);
        close(fds[READ]);
        execvp(cmd_left[0], cmd_left);
        perror("execvp failed\n");
    }
    else {
        close(fds[0]);
        close(fds[1]);
        
        if(!background){
            waitpid(pid1, NULL, 0);
            waitpid(pid2, NULL, 0);
        }
    }
}

void RedirectCmd(char* part1, char* part2){
    char* cmd_left[20];
    char* cmd_right[20];
    int fds[2];
    int fd;
    int count;
    pid_t pid1, pid2;
    int background;
    char c;

    if( pipe(fds) < 0){
        perror("pipe failed\n");
        exit(1);
    } 


    background = Parse(part1, cmd_left) || 
                 Parse(part2, cmd_right);
   
 
    if( (pid1 = fork()) < 0){
        perror("fork failed\n");
        exit(1);
    } 

    if( pid1 == 0 ){
        dup2(fds[WRITE], 1);
        close(fds[READ]);
        execvp(cmd_left[0], cmd_left);
        perror("execvp failed\n");
        exit(1);
    }

    if( (pid2 = fork()) < 0){
        perror("fork failed\n");
        exit(1);
    }

    if( pid2 == 0 ){
        if( (fd = open(cmd_right[0], O_RDWR | O_CREAT | O_APPEND, 0600)) < 0){
            perror("open failed\n");
            close(fd);
            exit(1);
        }

        dup2(fds[READ], 0);
        close(fds[WRITE]);

        while( (count = read(0, &c, 1)) > 0){
            write(fd, &c, 1);
        }

        close(fd);

        /**** 追記２ : exit(1)がないと無限プロセスになるため追加 ****/
        exit(1);
    }
    else {
        close(fds[READ]);
        close(fds[WRITE]);
        waitpid(pid1, NULL, 0);
        waitpid(pid2, NULL, 0);
    }

}

int main(void){
    char cmd[256];
    char part1[128];
    char part2[128];
    int  type = -1;

    printf("\n> ");
    getCommand(cmd);

    while( (strcmp(cmd, "quit")!=0) &&
           (strcmp(cmd, "exit")!=0)    ){
        split(cmd, part1, part2, &type);

        if(type == 0){
            RegCmd(part1);
        }
        else if(type == 1){
            PipeCmd(part1, part2);
        }
        else if(type == 2){
            RedirectCmd(part1, part2);
        }

        #ifndef DEBUG
        printf("\nType = %d", type);
        #endif 

        printf("\n> ");
        getCommand(cmd);
    }
    return 0;
}

 

Comments

[kazto]

getCommand(cmd)関数の内容を貼り付けて下さい。

[cateye]

getCommand()ってWindows PowerShellでしょうか？　・・・違うなら、getCommand()の中身等情報不足です。

[a_saitoh]

「無限ループになってしまう」のは一体どういう根拠ですか？観察出来る挙動そのものを書いてください。デバッガで追いかけると無限ループしてるのか、待っても終わらないので無限ループだとおもった、のか。

[ijuya_yika]

ご指摘ありがとうございます、コード修正しました。また無限ループ => 無限にプロセスが作成されてしまうに変更しました。

[a_saitoh]

本当にプロセス一覧を見てプロセス無限生成と確認しましたか？本当にそうなら最悪PCが操作不能になってしまいそうなものですが。

Answer 1

自作シェルを作るというのはきっと色々勉強になると思います。

しかしながら自分でデバッグしないのでは何にもなりません。

デバッグしたならそれがご質問に書かれているはずですが、全然書かれてないので「デバッグしてないのだろう」と判断しました。デバッグしてください。

---

最も基本的な誰でもが最初にやるデバッグは「デバッグプリント」だと思います。
getCommandの結果はcmd, p1, p2に期待どおり設定されているか？
execvpには正しい文字列が渡っているか？
whileループの部分で、無限にループしているのか、ループの途中でreadで止まっているのか、読めている文字は何か、等々。

こういうのはデバッグプリントをちょこっと入れてやると「事実」がつかめます。ソースを見てもわからないときは「どう動いているかの事実」を掴んでそこから逆に原因を探りましょう。

---

無限ループになっている

と書いておられますが本当に無限ループになっていますか？コマンドプロンプトが表示されないためそれを「無限ループ」と言っているだけではないのですか？

「プログラムが反応しない」ことを「無限ループ」とは言いません。例えば「次のプロンプトが表示されない」というべきです。本当に無限ループしているのかも知れませんが、単にreadなどのシステムコールが戻ってきてないだけかも知れません。

プログラマーというのはこういう「一見細かい違いに見える」言葉にも非常に敏感です。用語が違うと認識がずれ、それがバグを作り込んだりバグの原因を正しく把握できない原因になるからだろうと思います。

Comments

[ijuya_yika]

コメントありがとうございます。質問及びコードが大雑把すぎたので修正しました。execvp等に入る文字列が正しいことはデバッグコードで確認済です。

[KSwordOfHaste]

質問文を追加し大分論点がはっきりしてきたと思います。ただ・・・
part2が空文字列になってしまってます。これは期待ではないように思いますが？

[ijuya_yika]

>> が入力されない場合はpart2は空、入力された場合はpart2に>>の右側にファイル名が入り、main関数のif文で>>が入力された時とされない時で処理を分岐してますけど、part2が空の場合なにか問題ありますか？

[KSwordOfHaste]

ls>>f
とやるとpart1が"ls",part2が"f"になるのが期待なんじゃないかなぁと思ったのです。実際はこのケースでpart2が""になるように思います。論理を詳しくみてませんが・・・
---
>>の両側にスペース文字が必要なんですね。これは本件とは関係なかったですね。

[KSwordOfHaste]

exitしない原因を探るヒントですが、プロンプトを表示するところで
printf("\n%d> ", getpid());
とやってみると核心にせまれると思います。
---追記---
ちょっと中途半端なコメントでした。「各デバッグプリントにgetpidの値も表示すると核心にせまれる」と言った方がよかったです。

[ijuya_yika]

コメントありがとうございます。　>> の左右にスペースが入ることを想定したコードだったのでスペース無しに対応するよう６０行目を修正しました。

Answer 2

根本的問題として、リダイレクトに誤解があるようです。

cmd>>file

の実装が

cmd |cat>file

的なものになってます。
リダイレクトを行うためのforkは必要ありません。

あと、

file　ならO_TRUNC　が要りますし

file　なら　O_APPEND

Comments

[ijuya_yika]

コメントありがとうございます。O_APPENDが抜けていましたので修正しました。fork無しの実装、差し支えなければ簡単な手順を訊いても良いでしょうか

[a_saitoh]

パイプがない場合はだいたいこんな感じ。
pid=fork()；
if(pid==0){
　　fd=open(filename,O_WRONLY|O_TRUNC｜O_CREAT)
　　fdを1に繋ぎ変えてfdをclose
　　exec(・・・)
　　エラー処理＆exit
}

Answer 3

while( (count = read(0, &c, 1)) > 0){
            write(fd, &c, 1);
        }

のreadでパイプを読み出すときにブロックされているのでは？なので親プロセスのwaitpid(pid2, NULL, 0);で返ってこない。

本質的には下記だけで十分だと思います。(嘘言ってたらごめんなさい。)

fd = open(Right[0],  O_WRONLY|O_CREAT|O_APPEND, 0666);
dup2(fd, 1);
close(fd);
execvp(Left[0], Left);

Comments

[nullbot]

おお、全く原因が違いましたね。お恥ずかしい。。。

なるほど。pid1で生成された子プロセスがpid2を子プロセスとして生成するので・・・という理由ですかね。さらに子プロセスをexitで終了させてなければmain関数のwhileループに戻り、子プロセスの方は標準入力がdupで差し替えられているので、getCommand内のscanfが期待通りには動作しないと。

Answer 4

    while( (count = read(0, &c, 1)) > 0){

これが意味不明ですね

---

    fd = open(Right[0], O_RDWR | O_CREAT, 0600)) < 0);

Right に何も入ってない

---

その他、このソースに提示されてない項目がたくさんありそーですね。

Comments

[cateye]

NULLポインタを渡してるように見えますが・・・なんでしょうね？
ファイルディスクリプタの番号、STDIN_FILENOが０なのでそうしてるように見えます。

[y_waiwai]

いつのまにか質問が変わってるw

[ijuya_yika]

すみません、質問が大雑把すぎて自分で読み返しても?だったのでかなり変更しました。