Handling arguments in a custom shell

Question

I am currently working on a shell, implemented in C, for class that I hope to build upon over time and have run into a problem with executing my arguments. My program uses getchar() to parse entries into the array of arguments and then executes the arguments using execvp(). The issue I have is in repeated entry of arguments any subsequent shorter arguments are concatenated with characters left in memory somewhere. Example below. And I am required to use getchar, which rules out alternate methods of getting the arguments.

//Global Variables    
char argument[64];  
char **argv;

int main(int argc, char** argv) {
    mainloop();                      //Loop that calls the commands
    return (EXIT_SUCCESS);
}

void prompt(){                       //Modular prompt
    printf ("?:");
}

void mainloop(){                    //Loop that calls the functions
    while(1){
        prompt();
        argv = (char**)malloc(sizeof(char*)*64);  //allocate memory for argv
        getcommand(argument, argv);
        if((strcmp(argv[0],"exit" )) == 0){  //check for exit
            return 0;
        }
        executecommand();
        printcommand();
        //clearcommand();
        //printcommand();
    }
}

void getcommand(char* argument, char** argv){  //Parser for the command
    int i=0,j=0;
    char c;
    char* token;
    while((c = getchar()) != '\n' ){ //gets char and checks for end of line
        argument[i] = c;
        i++;
    }
    token = strtok(argument, " ,.");  //tokenize the command
    while (token != NULL){ 
        argv[j] = token;   //pass command to array of arguments
        token = strtok(NULL, " ,.");
        j++;
    }
    //argv[j] = "\0";
}

void executecommand(){  //Function to call fork and execute with errors
    pid_t childpid = fork();
    int returnStatus;
    if(childpid == -1){                           //Fail to Fork
        printf("failed to fork");
        exit(1);
    }
    else if(childpid == 0){                      //Child process
        if (execvp(*argv, argv) < 0){
            printf("error executing\n");
            exit(1);
        }
        else{                                   //Execute successful
            printf("executed");
        }
    }
    int c=(int)waitpid(childpid, &returnStatus, 0);
    if (returnStatus == 0)  // Verify child process terminated without error.  
        {
            printf("The child process terminated normally. \n");   
        }

    if (returnStatus == 1)      
        {
            printf("The child process terminated with an error!.\n");    
        }
    //realloc(argv,64);
}

void printcommand(){  //Test function to print arguments
    int i = 0;
    while(argv[i] != NULL){
        printf("Argv%d: %s \n",i, argv[i] );
        i++;
    }
}

/*void clearcommand(){     //Function to clear up the memory, does not work
  int i=0;
  argv[0] = "       \0";
  argv[1] = "       \0";

  }*/

Example output:

?: ls -l
//functions as intended

?:echo
//argument returns as echol

This is the case for any entry which is shorter than a previous entry. I do not understand why exec is reading the argument continuing after a '\0' and i am sure that I am making a memory error here. Help would be very much appreciated, I have been stuck on this one for a couple of days now.

I've edited your question. Your reference to a "C shell" could be confusing; it commonly refers to the existing csh shell. — Keith Thompson
– Keith Thompson, Commented Sep 30, 2015 at 23:08
Use prototypes! And don't cast the result of malloc & friends in C! — too honest for this site
– too honest for this site, Commented Oct 1, 2015 at 0:24
I removed the global variables as per your recommendation, and Thanks Keith, I was not aware of that! — Chuckles
– Chuckles, Commented Oct 1, 2015 at 0:40

Barmar · Accepted Answer · 2015-09-30 22:52:51Z

3

You need to indicate the end of the argv array with an element that contains a null pointer. The commented-out line:

argv[j] = "\0";

should be:

argv[j] = NULL;

You also need to put a null terminator at the end of the argument string. You're getting l when you do the echo because argument still contains the previous command line. So the first line sets argument to:

ls -l

Then you overwrite the first 4 characters with echo, so it becomes:

echol

So the full function would be:

void getcommand(char* argument, char** argv){  //Parser for the command
    int i=0,j=0;
    char c;
    char* token;
    while((c = getchar()) != '\n' ){ //gets char and checks for end of line
        argument[i] = c;
        i++;
    }
    argument[i] = '\0';
    token = strtok(argument, " ,.");  //tokenize the command
    while (token != NULL){ 
        argv[j] = token;   //pass command to array of arguments
        token = strtok(NULL, " ,.");
        j++;
    }
    argv[j] = NULL;
}

You could also use fgets() to read a line of input, instead of calling getchar() yourself.

You should also check for the input being larger than the size of argument.

edited Sep 30, 2015 at 22:52

answered Sep 30, 2015 at 22:47

Barmar

789k57 gold badges555 silver badges669 bronze badges

Sign up to request clarification or add additional context in comments.

1 Comment

Chuckles Over a year ago

Thank you for such a clear explanation! I was entirely overlooking argument as the problem.

user3386109 · Accepted Answer · 2015-09-30 22:49:45Z

3

After the loop that reads an input line, you need to terminate the line with a NUL character.

while((c = getchar()) != '\n' ){ //gets char and checks for end of line
    argument[i] = c;
    i++;
}
argument[i] = '\0';  // <<---- terminate the input line

You also need to do what @Barmar said, but that's a different issue than the one that you describe in the question.

answered Sep 30, 2015 at 22:49

user3386109

35k7 gold badges52 silver badges69 bronze badges

Collectives™ on Stack Overflow

Handling arguments in a custom shell

2 Answers 2

1 Comment

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

1 Comment

Comments

Your Answer

Sign up or log in

Post as a guest

Related