Problems with making a simple Unix shell

Question

I am trying to create a simple shell in Unix. I read a lot and found that everybody uses the strtok function a lot. But I want to do it without any special functions. So I wrote the code but I can't seem to get it to work. What am I doing wrong here?

void process(char**);
int arg_count;
char **splitcommand(char* input)
{
    char temp[81][81] ,*cmdptr[40];
    int k,done=0,no=0,arg_count=0;
    for(int i=0 ; input[i] != '\0' ; i++)
    {
        k=0;
        while(1)
        {
            if(input[i] == ' ')
            {
                arg_count++;
                break;
            }
            if(input[i] == '\0')
            {
                arg_count++;
                done = 1;
                break;
            }
            temp[arg_count][k++] = input[i++];
        }
        temp[arg_count][k++] = '\0';
        if(done == 1)
        {
            break;
        }
    }
    for(int i=0 ; i<arg_count ; i++)
    {
        cmdptr[i] = temp[i];
        cout<<endl;
    }
    cout<<endl;
}


void process(char* cmd[])
{
    int pid = fork();
    if (pid < 0)
    {
        cout << "Fork Failed" << endl;
        exit(-1);
    }
    else if (pid == 0)
    {
        cout<<endl<<"in pid";
        execvp(cmd[0], cmd);
    }
    else
    {
        wait(NULL);
        cout << "Job's Done" << endl;
    }
}


int main()
{
    cout<<"Welcome to shell !!!!!!!!!!!"<<endl;
    char input[81];
    cin.getline(input,81);
    splitcommand(input);
}

Answer 1

strtok isn't really a special function as it's standard function in standard include string.h, so there is no good reason not to use it.

Answer 2

If you decide to make your shell more complicated then you may reason about using a tool for lexical analysis.

For example:

http://en.wikipedia.org/wiki/Flex_lexical_analyser

Answer 3

Several things:

• you don't return anything from the splitcommand function
• everything you do in the splitcommand function is done in local variables, so it (esp. the strings you make) will not survive its end
• the code that attaches the null terminator is wrong (you put it to the following string, not the current one)
• using fixed-size buffer is a great choice; people love it
• note that in real UNIX shells, not every space designates an argument, and not every argument is designated by spaces

I'd suggest you use strings and some (real) parser framework, provided it is not too special for you.

Answer 4

The problem is with the

arg_count++;  

inside the if(input[i] == ' ') and if(input[i] == '\0')

when you are parsing the command line and you find a space or you reach the end of the command line you are increment arg_count before you put a \0 at the end of the command you were reading.

So change it to:

        if(input[i] == ' ')  
        {  
            // arg_count++;  REMOVE THIS.
            break;  
        }  
        if(input[i] == '\0')  
        {  
            // arg_count++;  REMOVE THIS.
            done = 1;  
            break;  
        }  
        temp[arg_count][k++] = input[i++];  
    }  
    temp[arg_count][k++] = '\0'; // add null-char at the end. 
    arg_count++; // increment should happen here.

More bugs:

• You are not returning anything from splitcommand
• You cannot just return cmdptr because they point to local char arrays(temp) which will not persist after the function returns. So you'll have to make sure that the array temp persists even after function call by allocating it dynamically or making it global.
• Arguments to execvp look good to me. Others please take a look.

Answer 5

This is almost certainly homework. There is no reason to avoid library functions unless you were told to. In fact, most likely you were told to implement strtok.