Thread: Enigma execpvp

  1. #1
    Registered User
    Join Date
    Mar 2009
    Location
    Bozen
    Posts
    95

    Enigma execpvp

    Hello,

    I'm writing a mini shell in C and have problems with my code that I don't understand.

    Code:
    #include <stdio.h>
    #include <stdlib.h>
    #include <sys/types.h>
    #include <signal.h>
    #include <stdbool.h>
    #include <string.h>
    #include <unistd.h>
    #define maxLine 80
    
    void execute(char**args){
        pid_t pid;
        if ((pid = fork()) < 0) { perror("fork"); exit(1);}
        if (pid == 0){  if (execvp(*args, args) == -1){ perror("executing command\n"); exit(1);}}
        else wait(&pid);
    }
    void parse(char* input, char** args){
    	int argc = 0; char* delm = " \t\n"; char* token = strtok(input,delm);
    	while (token != NULL){
    		args[argc] = token; argc++;
    		token = strtok(NULL, delm);
    	}  execute(args);
    }
    int main(){
        char buf[maxLine];
        char *args[64];
    
        while(1){
            printf("Prompt: ");
            if (fgets(buf,maxLine, stdin) == NULL) {
                printf("\n");
                exit(0);
            } parse(buf, args);
        }
    }
    This code would work if:
    - I remove the the if condition around the excevp call, and move the execute call from the parse to the main. Why, I'd like to know.
    Last edited by simpatico_qa; 05-11-2009 at 01:11 PM.

  2. #2
    Registered User
    Join Date
    Oct 2008
    Location
    TX
    Posts
    2,059
    For starters execvp() never returns; it returns only if there is an error with an exit status of -1 to the environment so it should really be coded as
    Code:
    if (pid == 0){if (execvp(*args, args) == -1) {perror("executing command\n"); exit(1);}}

  3. #3
    and the hat of int overfl Salem's Avatar
    Join Date
    Aug 2001
    Location
    The edge of the known universe
    Posts
    39,660
    It would help if it wasn't so badly formatted.
    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. #4
    Registered User
    Join Date
    Mar 2009
    Location
    Bozen
    Posts
    95
    I did that modification before seeing your post but still no difference.

    Salem, clearly there are different programming prospectives. Formatted like this I see it focus more on the 'important' lines of code, and reduce scroll-blindness. But anyway this is not the topic of the thread, so u are welcome to 'properly' format it for your best understanding of it.

    The problem remains as of why when I include execute inside the parse, as shown in the code I get the following error when I launch a process that would otherwise work:

    executing command
    : Bad address
    Last edited by simpatico_qa; 05-11-2009 at 01:28 PM.

  5. #5
    Registered User
    Join Date
    Oct 2008
    Location
    TX
    Posts
    2,059
    Well it certainly would help if it were formatted as previously noted. Post the errors you're getting and the code that works.

  6. #6
    Registered User
    Join Date
    Mar 2009
    Location
    Bozen
    Posts
    95
    Code:
    void execute(char**args){
        pid_t pid;
        if ((pid = fork()) < 0) { 
              perror("fork");
             exit(1);
        }
        if (pid == 0){  
           if (execvp(*args, args) == -1){ 
               perror("executing command\n"); 
               exit(1);
            }
         }
        else wait(&pid);
    }
    void parse(char* input, char** args){
    	int argc = 0; 
            char* delm = " \t\n"; 
            char* token = strtok(input,delm);
    	while (token != NULL){
    		args[argc] = token; argc++;
    		token = strtok(NULL, delm);
    	}
    }
    int main(){
        char buf[maxLine];
        char *args[64];
    
        while(1){
            printf("Prompt: ");
            if (fgets(buf,maxLine, stdin) == NULL) {
                printf("\n");
                exit(0);
            } 
            parse(buf, args);
             execute(args);
        }
    }
    This code works. Compare with the previous posting where the execute statement is moved, and that simple movement gives the error msg pasted above:

    executing command
    : Bad address

  7. #7
    and the hat of int overfl Salem's Avatar
    Join Date
    Aug 2001
    Location
    The edge of the known universe
    Posts
    39,660
    > Formatted like this I see it focus more on the 'important' lines of code
    Yeah, at the expense of being able to figure out the program logic.
    Which is exactly the hole you've dug yourself into.

    Code is read a hell of a lot more than it is written (once if you're really good).
    Writing code so that it's easy to parse for the reader with minimal effort is what counts.

    With your better code, this took about 5 seconds to figure out.

    args[argc] = NULL;
    is what you need when you've finished parsing the arguments.
    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. #8
    Registered User
    Join Date
    Oct 2008
    Location
    TX
    Posts
    2,059
    Note that the second argument to execvp() is an array of pointers to char strings with the last one ie args[argc] being a NULL pointer, as in
    Code:
    while (token != NULL) {
        args[argc] = token;
        argc++;
        token = strtok(NULL, delm);
        args[argc] = NULL;
    }
    Just my 2c, but I tried both versions of the code and had no problems whether execute() was called from main() or parse().

  9. #9
    Registered User
    Join Date
    Mar 2009
    Location
    Bozen
    Posts
    95
    thank u very much Salem! May you complete my info pointing out why this extra line of code is required? Is it to stop the exec function from taking args from values that i actually didn't fill in (not initialized)?

  10. #10
    Registered User
    Join Date
    Oct 2008
    Location
    TX
    Posts
    2,059
    Without that last bit execvp() doesn't know where the list of arguments ends and moves onto the next pointer which could contain a bad/garbage value(address).

  11. #11
    and the hat of int overfl Salem's Avatar
    Join Date
    Aug 2001
    Location
    The edge of the known universe
    Posts
    39,660
    execv - Man Pages at IceWalkers.com
    The execv and execvp functions provide an array of pointers to null-
    terminated strings that represent the argument list available to the
    new program. The first argument, by convention, should point to the
    file name associated with the file being executed. The array of point-
    ers must be terminated by a NULL pointer.
    It's like mentioned 4 times, did you read it even once?
    Or just guess from the prototype?
    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.

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Win32 Battleship game enigma?
    By Finchie_88 in forum Windows Programming
    Replies: 7
    Last Post: 03-20-2005, 07:17 AM
  2. a new enigma
    By caroundw5h in forum A Brief History of Cprogramming.com
    Replies: 7
    Last Post: 05-17-2004, 07:14 PM
  3. Help About The ENIGMA Algoritm
    By JohnnyAtomic in forum C Programming
    Replies: 4
    Last Post: 02-22-2002, 10:05 AM
  4. Another do-while enigma..for me
    By bugeye in forum C Programming
    Replies: 9
    Last Post: 01-26-2002, 09:24 AM