Using execvp with dup2 raises EFAULT error

Question

I'm building a small linux shell and am trying to implement the > operator to redirect the output of the commands to a file.

The issue I have is that when I try to run something like ls > test.txt, I get a Bad Address (EFAULT) error.

However, if I try it without the redirection, everything works as expected.

I've trimmed the code to a minimum to test only for ls, but i still get the same error, here's the code.

int saved_stdout;
__pid_t id = fork();

if (id == 0) {
    saved_stdout = dup(1);

    int fd = open("test.txt", O_RDWR | O_CREAT, S_IRUSR | S_IWUSR);

    dup2(fd, STDOUT_FILENO);
    dup2(fd, STDERR_FILENO);

    close(fd);

    char* args[] = {"\0"};

    execvp("ls", args);

    fprintf(stderr, "Value of errno: %d\n", errno);
    perror("Error printed by perror");

} else {
    int status;
    waitpid(id, &status, 0);

    if (saved_stdout) {
        dup2(saved_stdout, 1);
        close(saved_stdout);
    }
}

Does anyone as an idea on what I'm doing wrong here?

Thanks a lot

Answer 1

Does anyone as an idea on what I'm doing wrong here?

The main problem is likely that your arguments to execvp() are incorrect:

    char* args[] = {"\0"};

    execvp("ls", args);

There are two things definitely wrong with that:

1. The argument array needs to be terminated with a null pointer. "\0" is not a null pointer; rather it is an array containing two null characters, and it decays to a valid, therefore non-null, pointer.

2. Even if "\0" were a null pointer, you would be short one argument. The first element of the argument vector, at index 0, should be a pointer to a string representing the program name.

In other words:

 char* args[] = { "ls", NULL };

 execvp("ls", args);

Additionally, the redirection you are performing is not consistent with the POSIX shell's treatment of the > redirection operator. In that form, that operator redirects only the standard output, not the standard error. Furthermore, it should open the designated file write-only, not read/write, because writing to it is all the program needs to do. Opening it read/write could cause it to fail to redirect to an existing file on which the user has write permissions but not read permissions.

Furthermore, The file mode you specify for the event that a new file is created also produces behavior inconsistent with the POSIX shell's. You should specify read/write permissions for user, group, and other, and let that be modified according to the umask in effect:

int fd = open("test.txt", O_WRONLY | O_CREAT,
    S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH | S_IWOTH);