-2

So this code terminates after a pipe command: for example cat text.txt | wc. It can open the text file and count the words but, instead of returning to the menu, it terminates. Any ideas?

#include <stdio.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <signal.h>
#include <dirent.h>
#include <fcntl.h>
#include <ctype.h>
#include <time.h>
#include <unistd.h>
#include <sys/stat.h>
#define LOGOUT 20
#define MAXNUM 40
#define MAXLEN 160
#define MAXSIZE 100
#define BUFSIZE 20
#define TRUE 1
#define FALSE 0


typedef struct command_history
{
    char *command;
    struct command_history *next;
} history;

/* Main */

int main(void)
{
    char *currentpath, *cmd, line[MAXLEN], *args[MAXNUM], *args2[MAXNUM], *args3[MAXNUM];
    int background, i, j, t, wildcard, opts;
    int redir_in, redir_out;
    int fd;
    long size;
    char *buf;
    pid_t pid, pid2;

    char *history[MAXSIZE], command[MAXSIZE], command_number[MAXSIZE], *temp;
    int history_count = 0, length, number, offs;
    int pipefd[2], pipefd2[2];
    int numberofpipes, pipesuccess;
    char buffer[BUFSIZE];
    FILE *fp;

    buf = (char*)malloc((size_t)size);

    /* set the signal handler for alarm */

    /*get default working directory as current path*/
    currentpath = getenv("PWD");

    while (1)
        {
            /* initialize */

            background = FALSE;
            opts = FALSE;
            redir_in = FALSE; redir_out = FALSE;
            pipesuccess = FALSE;
            numberofpipes = 0, t = 0, offs = 0;

            /* print the prompt */
            fprintf(stdout, "%s > ", currentpath);

            /* set the timeout for autologout, function alarm() */

            /* read the users command */
            if (fgets(line,MAXLEN,stdin) == NULL) {
                fprintf(stdout, "\nlogout\n");
                exit(0);
            }

            line[strlen(line) - 1] = '\0';

            if (strlen(line) == 0)
                continue;

            /* start to background? (check if the last character is '&') */
            if(line[strlen(line)-1] == '&')
                {
                    line[strlen(line)-1] = '\0'; //remove '&'
                    background = TRUE;
                }

            /* saving the user command into history list */

            if(line[0] != '!'){
                if (history_count < MAXSIZE){
                    history[history_count] = strdup(line);
                    history_count++;
                }
            }


            /* split the command line to args[]*/
            i = 0;  //number of arguments
            cmd = line;
            while((args[i] = strtok(cmd, " ")) != NULL)
                {
                    i++; //argument count
                    cmd = NULL;
                }

            /* history usage */
            if(line[0] == '!')
                {
                    // find right command index from history
                    j = 0;
                    while (isdigit(line[j+1])){
                        command_number[j] = line[j+1];
                        j++;
                    }
                    number = atoi(command_number);
                    if (number <= history_count)
                        {
                            // parsing the history commands back into args
                            t = 0;
                            temp = history[number-1];
                            while((args[t] = strtok(temp, " ")) != NULL)
                                {
                                    t++;
                                    temp = NULL;
                                }
                        }
                    else
                        {
                            printf("Out of range of array.\n");
                        }
                }

            /* find and open redirected files*/
            for(j = 0; j < i; j++)
                {
                    if(strcmp(args[j], ">") == 0){
                        args[j] = NULL;
                        fd = open(args[j+1], O_WRONLY | O_CREAT, S_IRUSR | S_IWUSR);
                        redir_out = TRUE;
                        if (fd < 0) {
                            perror("open");
                            exit(0);
                        }
                    }
                    else if(strcmp(args[j], "<") == 0){
                        args[j] = NULL;
                        fd = open(args[j+1], O_RDONLY, S_IRUSR | S_IWUSR);
                        redir_in = TRUE;
                        if (fd < 0) {
                            perror("open");
                            exit(0);
                        }
                    }
                }

            /* exit? */
            if (strcmp(args[0], "exit") == 0) {
                exit(0);
            }

            /*  cd  */
            if (strcmp(args[0], "cd") == 0)
                {
                    if(i > 1)
                        {
                            if(chdir(args[1]) != 0) //change to inputted directory
                                {
                                    perror("chdir");
                                }
                            currentpath = getcwd(buf, (size_t)size); //set currentpath to cwd
                        }
                    else
                        {
                            currentpath = getenv("HOME"); // change to homedir
                        }

                    continue;
                }

            /* history */
            if(strcmp(args[0], "history") == 0)
                {
                    printf("Command history: \n");
                    for (j = 0; j < history_count; j++) {
                        printf("[%d]  %s \n", j+1, history[j]);
                    }
                    continue;
                }

            /* find pipe marks for single and double pipe*/
            int k=0, z=0, y=0;
            if(!redir_out && !redir_in){
                for (j = 0; j < i; j++) //i == argument count
                    {
                        if(strncmp(args[j], "|", 1) == 0){
                            args[j] = NULL; // make it null, so exec will stop there
                            numberofpipes++;
                            continue;
                        }
                        if(numberofpipes == 1){
                            args2[z] = args[j]; //copy other part of args to args2
                            args2[z+1] = NULL; //make sure last args2 is (null)
                            z++;
                            /*  continue; */
                        }
                        /* if(numberofpipes == 2){
                           args3[y] = args[j]; //copy other part of args to args3
                           args3[y+1] = NULL; //make sure last args3 is (null)
                           y++;
                           } */
                    }
            }

            /* single pipe */
            if(numberofpipes == 1){
                pipe(pipefd);
                if((pid = fork()) == -1){
                    perror("fork");
                    exit(1);
                }
                if(pid == 0) { /* child, args2 */
                    close(pipefd[0]);
                    dup2(pipefd[1], STDOUT_FILENO);
                    close(pipefd[1]);
                    execvp(args[0], args);
                }
                else {  /* parent, args */
                    close(pipefd[1]);
                    dup2(pipefd[0], STDIN_FILENO);
                    close(pipefd[0]);
                    execvp(args2[0], args2);
                    perror("execv");
                }
            }
            /* double pipe */
            else if(numberofpipes == 2){
                pipe(pipefd);
                pipe(pipefd2);

                if((pid = fork()) == -1){
                    perror("fork");
                    exit(1);
                }
                if(pid == 0) {
                    if((pid2 = fork()) == -1){
                        perror("fork 2");
                        exit(1);
                    }
                    if(pid2 == 0){ /* grandchild, args3 */
                        close(pipefd[0]);
                        close(pipefd[1]);
                        close(pipefd2[1]);
                        dup2(pipefd2[0], STDIN_FILENO);
                        close(pipefd2[0]);
                        execvp(args3[0], args3);
                        perror("execv");
                    }
                    else { /* child, args2 */
                        close(pipefd[1]);
                        dup2(pipefd[0], STDIN_FILENO);
                        close(pipefd[0]);
                        close(pipefd2[0]);
                        dup2(pipefd2[1], STDOUT_FILENO);
                        close(pipefd2[0]);
                        execvp(args2[0], args2);
                        perror("execv");
                    }
                }
                else{ /* parent, args */
                    close(pipefd[0]);
                    dup2(pipefd[1], STDOUT_FILENO);
                    close(pipefd[1]);
                    execvp(args[0], args);
                }
            }

            /* fork to run the command */
            switch (pid = fork()) {
            case -1:
                /* error */
                perror("fork");
                continue;
            case 0:/* child process, exec() */

                if(redir_out){ //redirection to output file
                    dup2(fd, STDOUT_FILENO);
                    close(fd);
                }
                else if(redir_in){ //redirection to input file
                    dup2(fd, STDIN_FILENO);
                    close(fd);
                }


                execvp(args[0], args);
                perror("execv");
                exit(1);

            default:
                /* parent (shell), wait() if not background process */
                if(!background){
                    alarm(0);
                    while(wait(NULL) != pid)
                        {
                            printf("please.");
                        }
                }
                break;
            }

        }
    //globfree(&globbuf);
    free(buf);
    return 0;
}
Jonathan Leffler
  • 730,956
  • 141
  • 904
  • 1,278
  • 1
    Questions seeking debugging help ("why isn't this code working?") must include the desired behavior, a specific problem or error and the shortest code necessary to reproduce it in the question itself. Questions without a clear problem statement are not useful to other readers. See: How to create a Minimal, Complete, and Verifiable example. – too honest for this site Feb 03 '17 at 23:03
  • I bet it has something to do with the line with the `exit` statement – bruceg Feb 03 '17 at 23:31

1 Answers1

2

Pass 1

Your single-pipe block of code is:

/* single pipe */
if(numberofpipes == 1){
    pipe(pipefd);
    if((pid = fork()) == -1){
        perror("fork");
        exit(1);
    }
    if(pid == 0) { /* child, args2 */
        close(pipefd[0]);
        dup2(pipefd[1], STDOUT_FILENO);
        close(pipefd[1]);
        execvp(args[0], args);
    }
    else {  /* parent, args */
        close(pipefd[1]);
        dup2(pipefd[0], STDIN_FILENO);
        close(pipefd[0]);
        execvp(args2[0], args2);
        perror("execv");
    }
}

Your parent process executes one of the two parts of the pipeline; of course it terminates when the pipeline terminates.

You'll need to fork once more, somewhere along the line. You can either have the parent process (main shell) fork each child in turn, or you can have the parent fork once, and then the first child undertakes creating the pipeline. The advantage of the first is that the parent shell can find the exit status of each process in the pipeline — it's what Bash does. The advantage of the second is that it is more like what old shells did. The first child process executes the last process in the pipeline, and its exit status controls the exit status of the pipeline as a whole.

With the second method, the first child creates the pipe(s) and the parent doesn't have to worry about closing them. With the first method, the parent process must ensure that it closes pipes before it waits for anything to exit.


Pass 2

I edited it like so http://pastebin.com/yyMBnKPu Now it doesn't terminate but runs cat a second time and then hangs.

What compilation options do you use? I always compile my C code fussy, and end up compiling answers for SO with fussy options too. With the code from PasteBin (470-odd lines — it is reasonable not to include all that in the question), I get:

$ gcc -O3 -g -std=c11 -Wall -Wextra -Werror -Wmissing-prototypes -Wstrict-prototypes \
>     -Wold-style-definition sh61.c -o sh61 
sh61.c:30:6: error: no previous prototype for ‘sighandler’ [-Werror=missing-prototypes]
 void sighandler(int sig)
      ^~~~~~~~~~
sh61.c: In function ‘sighandler’:
sh61.c:30:21: error: unused parameter ‘sig’ [-Werror=unused-parameter]
 void sighandler(int sig)
                     ^~~
sh61.c: At top level:
sh61.c:38:6: error: no previous prototype for ‘left_child’ [-Werror=missing-prototypes]
 void left_child(char *args[MAXNUM], int pipefd[2])
      ^~~~~~~~~~
sh61.c:49:6: error: no previous prototype for ‘right_child’ [-Werror=missing-prototypes]
 void right_child(char *args2[MAXNUM],int pipefd[2])
      ^~~~~~~~~~~
sh61.c:59:5: error: no previous prototype for ‘SinglePipe2’ [-Werror=missing-prototypes]
 int SinglePipe2(char *args[MAXNUM], char *args2[MAXNUM], int redir_in, int redir_out, int fd){
     ^~~~~~~~~~~
sh61.c: In function ‘SinglePipe2’:
sh61.c:115:13: error: this ‘if’ clause does not guard... [-Werror=misleading-indentation]
             if (execvp(args[0], args))
             ^~
sh61.c:117:5: note: ...this statement, but the latter is misleadingly indented as if it is guarded by the ‘if’
     exit(1);
     ^~~~
sh61.c: In function ‘main’:
sh61.c:367:19: error: "/*" within comment [-Werror=comment]
    if(pid == 0) { /* child, args2

sh61.c:374:26: error: "/*" within comment [-Werror=comment]
    else {  parent, args  /*

sh61.c:397:21: error: "/*" within comment [-Werror=comment]
      if(pid2 == 0){ /* grandchild, args3

sh61.c:406:13: error: "/*" within comment [-Werror=comment]
      else { /* child, args2

sh61.c:417:10: error: "/*" within comment [-Werror=comment]
    else{ /* parent, args

sh61.c:428:5: error: "/*" within comment [-Werror=comment]
     /* error

sh61.c:431:11: error: "/*" within comment [-Werror=comment]
    case 0:/* child process, exec()

sh61.c:456:5: error: "/*" within comment [-Werror=comment]
     /* parent (shell), wait() if not background process

sh61.c:332:7: error: unused variable ‘k’ [-Werror=unused-variable]
   int k=0, z=0, y=0;
       ^
sh61.c:142:8: error: unused variable ‘fp’ [-Werror=unused-variable]
  FILE *fp;
        ^~
sh61.c:141:7: error: unused variable ‘buffer’ [-Werror=unused-variable]
  char buffer[BUFSIZE];
       ^~~~~~
sh61.c:140:21: error: variable ‘pipesuccess’ set but not used [-Werror=unused-but-set-variable]
  int numberofpipes, pipesuccess;
                     ^~~~~~~~~~~
sh61.c:139:17: error: unused variable ‘pipefd2’ [-Werror=unused-variable]
  int pipefd[2], pipefd2[2];
                 ^~~~~~~
sh61.c:139:6: error: unused variable ‘pipefd’ [-Werror=unused-variable]
  int pipefd[2], pipefd2[2];
      ^~~~~~
sh61.c:138:25: error: unused variable ‘length’ [-Werror=unused-variable]
  int history_count = 0, length, number, offs;
                         ^~~~~~
sh61.c:137:26: error: unused variable ‘command’ [-Werror=unused-variable]
  char *history[MAXSIZE], command[MAXSIZE], command_number[MAXSIZE], *temp;
                          ^~~~~~~
sh61.c:136:19: error: unused variable ‘act’ [-Werror=unused-variable]
  struct sigaction act;
                   ^~~
sh61.c:130:6: error: variable ‘background’ set but not used [-Werror=unused-but-set-variable]
  int background, i, j, t, wildcard, opts;
      ^~~~~~~~~~
sh61.c:129:73: error: variable ‘args3’ set but not used [-Werror=unused-but-set-variable]
  char *currentpath, *cmd, line[MAXLEN], *args[MAXNUM], *args2[MAXNUM], *args3[MAXNUM];
                                                                         ^~~~~
cc1: all warnings being treated as errors
$

The problem at line 115ff is:

        if (execvp(args[0], args))
            perror("execv");
            exit(1);

The perror() is controlled by the if but the exit() occurs unconditionally. However, since execvp() only returns if it fails, there is no need for the test:

        execvp(args[0], args);
        perror("execv");
        exit(1);

Unused variables are a pain. Badly nested comments are a bad sign. If you want to eliminate blocks of code, use #if 0 before and #endif after them. Using the functions is a good idea. Using a VCS (version control system) is a good idea. Using comments to eliminate blocks of code is a bad idea. With the commented out code gone, the file is down to 370 or so lines.

The undeclared functions warnings come from my insistence on strict prototypes etc. All functions except main() are static (only accessible from the file they're written in) unless there's a header that declares them, and the header is used in the file where the function is defined and all the files where the function is used. Personally, I regard this as highly beneficial.

Dealing with unused variables is mostly simple. Note that you're not supposed to use printf() in a signal handler.

With those issues fixed, there is a problem that you've commented out the code to set redir_in and redir_out to a known value (0 or FALSE) so the pipe-hunting code is not necessarily entered. And with that fixed, you have code in SinglePipe2() that looks like:

    close(pipefd[0]);       /* close parent's copy of pipe */
    close(pipefd[1]);

    while ((ret = wait(&status)) > 0)       /* wait for children */
    {
        if (ret == left_pid)
            printf("left child %d terminated, status: 0x%.4X\n", ret, status);
        else if (ret == right_pid)
            printf("right child %d terminated, status: 0x%.4X\n", ret, status);
        else
            printf("yow! unknown child %d terminated, status %x\n",
                   ret, status);
    }

#if 0
    switch (pid = fork())
    {
    case -1:

        perror("fork");

    case 0:        /* child process, exec() */

        if (redir_out)         // redirection to output file
        {
            dup2(fd, STDOUT_FILENO);
            close(fd);
        }
        else if (redir_in)         // redirection to input file
        {
            dup2(fd, STDIN_FILENO);
            close(fd);
        }

        execvp(args[0], args);
        perror("execv");
        exit(1);
    }
#endif /* 0 */

Except your non-working code doesn't have the #if 0 and #endif lines commenting out the code which causes the misbehaviour you see. Remove that block of code.


Future issues

Note that your redirection handling code is not adequate. It is legitimate to write:

cat < sh61.c | wc -l > wc.out

which has input redirection on the left-hand child and output redirection on the right-hand child process. However, that is a future issue. Your MCVE (Minimal, Complete, Verifiable Example) should not really include the 'history' handling and I/O redirection handling code — you aren't worried about those parts yet. Arguably, the cd command should have been left out too. I can see reasons for keeping the exit code, though the shell handles EOF cleanly too.

Community
  • 1
  • 1
Jonathan Leffler
  • 730,956
  • 141
  • 904
  • 1,278
  • Alrighty I edited it like so http://pastebin.com/yyMBnKPu Now it doesn't terminate but runs cat a second time and then hangs. – user7513618 Feb 05 '17 at 21:09