#1
  1. No Profile Picture
    Registered User
    Devshed Newbie (0 - 499 posts)

    Join Date
    Mar 2013
    Posts
    5
    Rep Power
    0

    Issues running the following code


    I am trying to run the following code. It is suppose to be a shell. The user input is turned into a vector (an array of strings) then used.

    Code:
    int main()
    {
    	char **vector;
    	char buffer[256];
    	int pid,i;
    	
    	while(1)
    	{
    		my_str("Prompt:> ");
    
    		if((i = read(0, buffer, 255)) >= 0)
    		{
    			buffer[i] = '\0';
    			vector = str2vect(buffer);
    		}
    				
    		if(!strcmp(vector[0], "cd ") || !strcmp(vector[0], "cd"))
    		{
    
    			if(chdir(vector[1]) < 0)
    			{
    				my_str("Unknown directory");
    			}
    		}
    		
    		if(!strcmp(vector[0], "exit") || !strcmp(vector[0], "exit\n"))
    		{
    			exit(0);
    		}
    		else
    		{
    			if((pid = fork()) < 0)
    			{
    				my_str("Error");
    				exit(1);
    			}
    			else if(pid == 0)
    			{
    				if(execvp(vector[0],vector) != -1)
    				{
    					my_str("Command not found");
    					exit(0);
    				}
    			}
    		}
    	}
    }
    I'm not sure what the issue is. It seems to be going into an infinite loop somewhere. Any insight would be appreciated. Thanks for your help!
  2. #2
  3. Contributed User
    Devshed Specialist (4000 - 4499 posts)

    Join Date
    Jun 2005
    Posts
    4,392
    Rep Power
    1871
    How are we supposed to figure out 'somewhere' when you don't post all the code?

    Where are these implemented?
    - my_str
    - str2vect
    If you dance barefoot on the broken glass of undefined behaviour, you've got to expect the occasional cut.
    If at first you don't succeed, try writing your phone number on the exam paper
  4. #3
  5. No Profile Picture
    Registered User
    Devshed Newbie (0 - 499 posts)

    Join Date
    Mar 2013
    Posts
    5
    Rep Power
    0
    Originally Posted by salem
    How are we supposed to figure out 'somewhere' when you don't post all the code?

    Where are these implemented?
    - my_str
    - str2vect
    Both these methods have been thoroughly used and tested and produce the correct output 100% of the time.
  6. #4
  7. Contributed User
    Devshed Specialist (4000 - 4499 posts)

    Join Date
    Jun 2005
    Posts
    4,392
    Rep Power
    1871
    > Both these methods have been thoroughly used and tested and produce the correct output 100% of the time.
    And yet somehow, it doesn't work.

    We'd like to see them for several reasons
    a) you simply saying they work isn't good enough
    b) passing tests doesn't prove the absence of bugs, only that it compiles and does work for the test conditions
    c) with all the code, we could actually try and run it ourselves.

    But if you're not going to post the code, then you're the one doing all the debugging.

    Compile with debug (-g flag), run the program in gdb (search the web) and when it seems to get stuck hit ctrl-c and start looking around. It will be in your apparent infinite loop somewhere.
    If you dance barefoot on the broken glass of undefined behaviour, you've got to expect the occasional cut.
    If at first you don't succeed, try writing your phone number on the exam paper
  8. #5
  9. Contributing User

    Join Date
    Aug 2003
    Location
    UK
    Posts
    5,111
    Rep Power
    1803
    str2vect() may well work, but you have not told us what it actually does!? Some example test input may would also be useful.

    The code is fundamentally unsafe in any case, if the first condition:
    Code:
    if((i = read(0, buffer, 255)) >= 0)
    is false, the second condition:
    Code:
    if(!strcmp(vector[0], "cd ") || !strcmp(vector[0], "cd"))
    dereferences an uninitialized pointer, or at best the value from the previous iteration.

    strcmp() does not return a boolean, so the expression !strcmp( a, b ) is just bad form IMO; prefer strcmp( a, b ) == 0.

    Either way the most efficient method of debugging this code is to use a debugger rather than a forum! This will allow you to among other things step the code one line at a time and observe the variable state at each step.
  10. #6
  11. No Profile Picture
    Registered User
    Devshed Newbie (0 - 499 posts)

    Join Date
    Mar 2013
    Posts
    5
    Rep Power
    0
    Thanks for all the tips. I have adjusted per your suggestions and have pinpointed more specifically where it is going wrong.

    It looks like it may be an issue with the fork. After I typed in "exit" the exit(0) in the if statement executes but then the prompt kept printing endlessly. Like: "Prompt:> Prompt:> Prompt:> Prompt:> Prompt:> Prompt:> Prompt:> Prompt:> Prompt:>..."

    My current code is:
    Code:
    int main()
    {
    	char **vector;
    	char buffer[256];
    	int pid,i;
    	
    	while(1)
    	{
    		my_str("Prompt:> ");
    
    		if((i = read(0, buffer, 255)) >= 0)
    		{
    			buffer[i] = '\0';
    			vector = str2vect(buffer);
    		
    				
    			if(strcmp(vector[0], "cd") == 0)
    			{
    				if(chdir(vector[1]) < 0)
    				{
    					my_str("Unknown directory");
    					my_char('\n');
    				}
    			}
    		
    			if((strcmp(vector[0], "exit") == 0) || (strcmp(vect[0], "exit\n") == 0))
    			{
    				exit(0);
    			}
    			else
    			{
    				if((pid = fork()) < 0)
    				{
    					my_str("Error");
    					exit(1);
    				}
    				else if(pid == 0)
    				{
    					if(execvp(vector[0],vector) != -1)
    					{
    						my_str("Command not found");
    						exit(0);
    					}
    				}
    			}
    		}
    	}
    }
    requested my_str
    Code:
    void my_str(char *string)
    {
         if(string != NULL)
         {
              while(*string != '\0')
              {
                       my_char(*(string++));
              }
         }
    }
    
    void my_char(char c)
    {
         write(1,&c,1);
    }
    and str2vect
    Code:
    char **str2vect(char *str)
    {
        char** vect;
        char** vect_tmp;
        char* temp;
        int num_whitespaces = 0;
    
        temp = str;
    
        for(; *temp!= '\0'; temp++)
        {
            if(*temp == ' ')
            {
                num_whitespaces++;
            }
        }
    
        vect_tmp = (char **)malloc((num_whitespaces+1)*sizeof(char *));
        vect = vect_tmp;
    
        *vect = str;
    
        for(; *str != '\0'; str++)
        {
            if(*str == ' ')
            {
                 *str = '\0';
                 *(++vect) = str + 1;
            }
        }
    
        *(++vect) = NULL;
    
        return vect_tmp;
    }
    Thanks for all the help!
  12. #7
  13. Contributed User
    Devshed Specialist (4000 - 4499 posts)

    Join Date
    Jun 2005
    Posts
    4,392
    Rep Power
    1871
    > if(execvp(vector[0],vector) != -1)
    First off, exec functions return -1 on error, not -1 on success.
    So this test is completely inverted.

    Next,
    Code:
    Prompt:> /bin/ls
    
    Breakpoint 1, main () at foo.c:89
    89	        if((pid = fork()) < 0)
    (gdb) n
    Command not found94	        else if(pid == 0)
    (gdb) print pid
    $1 = 3075
    (gdb) print vector
    $2 = (char **) 0x602010
    (gdb) print vector[0]
    $3 = 0x7fffffffe080 "/bin/ls\n"
    (gdb) print vector[1]
    $4 = 0x0
    You need to strip off the \n as well, otherwise it's just going to come up with "command not found" all the time.

    Yes, this is a problem with your "debugged" str2vect

    What is also missing is something to free all the memory you allocated in str2vect as well.

    Finally, your parent process should wait for the child to exit, otherwise you print the prompt in the middle of the command output, like this.
    Code:
    Prompt:> /bin/ls
    Prompt:> file1.txt
    If you dance barefoot on the broken glass of undefined behaviour, you've got to expect the occasional cut.
    If at first you don't succeed, try writing your phone number on the exam paper
  14. #8
  15. No Profile Picture
    Registered User
    Devshed Newbie (0 - 499 posts)

    Join Date
    Mar 2013
    Posts
    5
    Rep Power
    0
    Originally Posted by salem
    > if(execvp(vector[0],vector) != -1)
    First off, exec functions return -1 on error, not -1 on success.
    So this test is completely inverted.

    Next,
    Code:
    Prompt:> /bin/ls
    
    Breakpoint 1, main () at foo.c:89
    89	        if((pid = fork()) < 0)
    (gdb) n
    Command not found94	        else if(pid == 0)
    (gdb) print pid
    $1 = 3075
    (gdb) print vector
    $2 = (char **) 0x602010
    (gdb) print vector[0]
    $3 = 0x7fffffffe080 "/bin/ls\n"
    (gdb) print vector[1]
    $4 = 0x0
    You need to strip off the \n as well, otherwise it's just going to come up with "command not found" all the time.

    Yes, this is a problem with your "debugged" str2vect

    What is also missing is something to free all the memory you allocated in str2vect as well.

    Finally, your parent process should wait for the child to exit, otherwise you print the prompt in the middle of the command output, like this.
    Code:
    Prompt:> /bin/ls
    Prompt:> file1.txt
    Thanks so much!! One question on the last part... I'm very new at forking. Where exactly would I put the wait()?
  16. #9
  17. Contributed User
    Devshed Specialist (4000 - 4499 posts)

    Join Date
    Jun 2005
    Posts
    4,392
    Rep Power
    1871
    Code:
    if((pid = fork()) < 0)
    {
        // error, no child process
    }
    else if(pid == 0)
    {
        // success, this is child
    }
    else
    {
        // success, this is parent
        // wait here!
    }
    If you dance barefoot on the broken glass of undefined behaviour, you've got to expect the occasional cut.
    If at first you don't succeed, try writing your phone number on the exam paper
  18. #10
  19. No Profile Picture
    Registered User
    Devshed Newbie (0 - 499 posts)

    Join Date
    Mar 2013
    Posts
    5
    Rep Power
    0
    Originally Posted by salem
    Code:
    if((pid = fork()) < 0)
    {
        // error, no child process
    }
    else if(pid == 0)
    {
        // success, this is child
    }
    else
    {
        // success, this is parent
        // wait here!
    }
    Ahh makes sense. Thanks for all your help!

IMN logo majestic logo threadwatch logo seochat tools logo