0

I have to program a little shell for school but I am stuck at even executing a command. execvp worked when I executed it in the wait for input function, but in the execute command function it doesnt e.g I don't get any output to stdout for commands like ls.

#include <stdio.h>
#include <unistd.h>
#include <sys/wait.h>
#include <errno.h>

int executecommand(char commandarray[]){
    char * command = &commandarray[0];
    pid_t pid = fork();
    if (pid < 0){
        perror("failed");
        return errno;
    } else if (0 == pid) {
        execvp(command, &commandarray);
        return 0;
    } else {
        int waitstatus;
        waitpid(pid, &waitstatus, 0);
        return 0;
    }
}

int waitforinput(){
    printf("$ ");
    char cmd[256];
    fgets(cmd, 256, stdin);
    executecommand(cmd);
    return 0;
}

int main(int argc, char **argv) {
    waitforinput();
    return 0;
}
Jocomol
  • 303
  • 6
  • 18
  • 1
    `commandarray` should be an array of pointers, not a single string. It becomes the `argv` array of the new process. – Barmar Jan 19 '23 at 09:57
  • 1
    You should check for errors from `execvp()`. – Barmar Jan 19 '23 at 09:59
  • @Barmar could you elaborate, and also how would I go on and fix this? – Jocomol Jan 19 '23 at 09:59
  • The string returned by `fgets()` ends with a newline. You need to remove that before trying to use it as a command, because the program name doesn't have a newline in it. – Barmar Jan 19 '23 at 10:00

2 Answers2

1

The argument to execvp() must be an array of strings ending with NULL, not a pointer to a single string. Normally you would parse the input line into an array containing the command followed by arguments, but if you don't care about arguments you can just create an array with 2 elements, the command followed by NULL.

You also need to remove the newline from the end of the input line returned by fgets(). See Removing trailing newline character from fgets() input for many ways to do this.

If you want to return errno, you need to save it before calling perror(), because that may change errno.

int executecommand(char *commandarray[]){
    char * command = commandarray[0];
    pid_t pid = fork();
    if (pid < 0){
        int saved_errno = errno;
        perror("fork");
        return saved_errno;
    } else if (0 == pid) {
        execvp(command, commandarray);
        int saved_errno = errno;
        perror("execvp");
        return saved_errno;
    } else {
        int waitstatus;
        waitpid(pid, &waitstatus, 0);
        return 0;
    }
}

int waitforinput(){
    printf("$ ");
    char cmd[256];
    char *cmd_array[2];
    fgets(cmd, 256, stdin);
    cmd[strcspn(cmd, "\n")] = 0; // remove trailing newline
    cmd_array[0] = cmd;
    cmd_array[1] = NULL;
    return executecommand(cmd_array);
}
Barmar
  • 741,623
  • 53
  • 500
  • 612
1

Invalid arguments to execvp :

From the man page:

int execvp(const char *file, char *const argv[])

The execv(), execvp(), and execvpe() 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 filename associated with the file being executed. > The array of pointers must be terminated by a NULL pointer.

The execvp function expects an array of strings terminated by a NULL pointer, not a pointer to one string.

Ignoring the return value of exec* and others:

The exec() functions only return if an error has occurred. The return value is -1, and errno is set to indicate the error.

Add a check for that.

Likewise, waitpid() may also fail, check its return value.

You also declared waitforinput() and executecommand() to return an int, but ignore the values returned. Either make use of them, or change the return type to void.

Trailing newline:

fgets(cmd, 256, stdin)

fgets will retain the newline in the buffer. Here's a one-liner that I use to remove it¹:

cmd [strcspn (cmd, "\n\r")] = `\0`;

Aside: Change

int main (int argc, char **argv)

to

int main (void)

as you never make use of the arguments.

[1] — Here are some others: https://codereview.stackexchange.com/q/67608/265278

Harith
  • 4,663
  • 1
  • 5
  • 20