IO redirection and buffer issues, fflush and c

Question

for my class we are to implement a shell with output redirection. I have the output redirection working, except my first command is always corrupted see:

$ echo this doesn't work
H<@?4echo
No such file or directory
$ echo this does work
this does work

but every command afterwards seems fine. What technique do I use to find the bug that is causing this problem?

I think it has something to do with not fflushing properly. I sprinkled it around my code (which was stupid) to see if it would help during the loop but it did not. I've also tried printing out my OrderedIds list which is just a list of commands to check if I could find H<@?4 anywhere, but even when I initialized it, it did not work.

Thanks for your help.

#define LENGTH 1000
#define MAXCMD 11
#define MAX_STR_LEN 20
void init(char *temp);
void clean(char **orderedIds);
void init_pid(int *temp);
void reap(int *temp,int ret_status); 
void jobs(int *pid_list, char **orderedIds);
int ioRedir(char **orderedIds);
void reap(int *temp,int ret_status){//chainsaws all zombies
    int a;  
    for (a=0; a<LENGTH; a++ ){
        waitpid(temp[a],&ret_status,WNOHANG) == temp[a];
    }
}
void init(char *temp){//Function to initialize/reset the cmd array
    int i;
    for(i=0; i<LENGTH; i++){
        temp[i] = 0; 
    }
}
void init_pid(int *temp){//Function to initialize/reset the pid list
    int i;
    for(i=0; i<LENGTH; i++){
        temp[i] = -777; 
    }
}
void clean(char **orderedIds){//garbage collection 
    int i; 
    for(i=0; i<MAXCMD; i++){
        free(orderedIds[i]);
    }
    free(orderedIds);
} 
void jobs(int *pid_list, char **orderedIds){//function to check for background proccesses
    printf("Jobs:\n");
    int y; 
    for(y=0; y<LENGTH; y++){
        if(kill(pid_list[y], 0) == 0){
            printf("%d\n", pid_list[y]); 
        }               
    }
    clean(orderedIds);
    printf("$ ");
}
int ioRedir(char **orderedIds){ 
    int i; 
    for ( i = 0; i<MAXCMD; i++){
        if(orderedIds[i] == NULL){
            return -1; 
        }
        if(strcmp(orderedIds[i],">")==0){
            return (i+1); 
        }

    }
}

int main (int argc, char *argv[], char *envp[])
{ 
    char temp[LENGTH];
    char * tok;
    char c = '\0';
    int saved_stdout;
    int pid_list[LENGTH];
    int ret_status;
    int numFile;
    int pid_counter = 0;
    int outputfd = -1;   
    char outputFile[MAX_STR_LEN]; 
    pid_t pid; 
    printf("$ ");
    int i, j, y, background= 0;  
    init_pid(pid_list); 
    while(c !=EOF) { //while not ^D // Source: LinuxGazzette Ramankutty
        outputfd = -1;
        fflush(0);
        c = getchar();    
        if(c=='\n'){ //entered command
            reap(pid_list, ret_status); 
            char **orderedIds = malloc(MAXCMD * sizeof(char*)); 
            for (i=0; i<MAXCMD; i++){
                 orderedIds[i] = malloc(MAXCMD * sizeof(char*)); 
            }
            int k=0; 
            tok = strtok(temp, " \n\t\r"); 
            while (tok !=NULL){ 
                strcpy(orderedIds[k], tok);
                k++;
                tok = strtok (NULL, " \n\t\r");
            }
            orderedIds[k] = NULL; //END with NULL 
            init(temp); //initialize the array
            if(orderedIds[0] ==NULL){
                printf("\n$ ");
                continue; 
            }
            numFile = ioRedir(orderedIds);
            if(strcmp(orderedIds[0],"exit")==0){// if exit
                printf("now exiting...\n"); 
                break;  
            }
            if(strcmp(orderedIds[k-1], "&")==0){//if background
                 orderedIds[k-1] = NULL; 
                 background = 1;
            }else background = 0; 

            if(strcmp(orderedIds[0], "jobs") == 0){//if jobs command    
                jobs(pid_list, orderedIds); 
                continue; 
            }   
            if(strcmp(orderedIds[0], "cd") == 0){ //if change directory command
                chdir(orderedIds[1]);
                printf("$ ");
                continue;
            }
            pid = fork();
            if (pid!=0 && background == 1)
            {
                //go to end of list in pid and put it in 
                pid_list[pid_counter] = pid; 
                pid_counter++; 
                printf("To the background: %d\n", pid);
            } else if (pid==0 && background == 1) {
                    fclose(stdin); //close child's stdin
                    fopen("/dev/null", "r"); //open a new stdin that is always empty.
                if(execvp(orderedIds[0], orderedIds)){
                    printf("%s\n", orderedIds[0]);
                    puts(strerror(errno));
                    exit(127); 
                }
            }
            if (pid != 0 && !background){
                //printf("Waiting for child (%d)\n", pid);
                fflush(0);
                pid = wait(&ret_status);
            }  else if (pid == 0 && !background) {
                    if(numFile > 0){
                        strncpy(outputFile, orderedIds[numFile], strlen(orderedIds[numFile])); 
                        numFile = 0;
                        //open the output file 
                        outputfd = open(outputFile, O_WRONLY | O_CREAT | O_TRUNC, S_IRWXU | S_IRGRP | S_IROTH); 
                        if (outputfd < 0) {
                            exit(EXIT_FAILURE);
                        }
                        //close STDOUT
                        if(close(STDOUT_FILENO) < 0 ){
                            perror("close(2) file: STDOUT_FILENO");
                            close(outputfd); 
                            exit(EXIT_FAILURE); 
                        }
                        //use dup to rerout the output
                        if(saved_stdout = dup(outputfd) != STDOUT_FILENO){
                            perror("dup(2)");
                            close(outputfd);
                            exit(EXIT_FAILURE);
                        }
                        close(outputfd);

                    }

                if (execvp(orderedIds[0], orderedIds)){
                    printf("%s\n", orderedIds[0]);
                    puts(strerror(errno));
                    exit(127);
                }
            }
            dup2(saved_stdout,outputfd);
            clean(orderedIds);
            fflush(0);
            printf("$ ");
        } else {
            strncat(temp, &c, 1); 
        }
    }
    fflush(0);
    return 0;    
}

Answer 1

The reason for the garbage is that you never initialized temp to an empty string at the beginning of main(). You call init(temp) after processing each command.

There are lots of other problems in your code:

orderedIds[i] = malloc(MAXCMD * sizeof(char*));

Since orderedIds[i] is an array of char, not char*, you should multiply the size by sizeof(char). Also, it's not clear why you're using MAXCMD as the size -- on the previous line this was the maximum number of words on a line, not the number of characters in a word.

strcpy(orderedIds[k], tok);

You should use strncpy() to ensure that you don't copy more than the size of orderedIds[k].

Another option would be not to preallocate all the orderedIds[i] in the first place. Instead of using strcpy(), use strdup() and assign this to orderedIds[k]; if you do this, you have to remember to free() all these strings.

A third option is not to copy the strings at all. Just assign the pointers returned by strtok() to orderedIds[k]. But in this case you mustn't call init(tmp) until after you've forked.

strncpy(outputFile, orderedIds[numFile], strlen(orderedIds[numFile])); 

The limit should be the size of outputFile, not the length of orderedIds[numFile]. strncpy() will never copy more than the length of the source, you need to tell it the maximum size of the destination to prevent a buffer overflow.

outputfd = open(outputFile, O_WRONLY | O_CREAT | O_TRUNC, S_IRWXU | S_IRGRP | S_IROTH); 
if (outputfd < 0) {
   exit(EXIT_FAILURE);
}

You should call perror() to report the reason that open() failed.

puts(strerror(errno));

Call perror(), like you do elsewhere.