What is the correct step to execute execve()?

Question

It is required to use pipe(), fork(), execve() and dup() to implement a simple execution of terminal command with pipe for our homework. So I read about how dup and pipe manipulate the file descriptor, and I produced the code below.

#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>

int main(void)
{
    int pfds[2];
    pipe(pfds);

    if (!fork()) {
        close(1);       /* close normal stdout */
        dup(pfds[1]);   /* make stdout same as pfds[1] */
        close(pfds[0]); /* we don't need this */
        char *k = {"echo", "one", "two", "three", NULL};
        execve("/bin/echo", k, NULL);
    } else {
        close(0);       /* close normal stdin */
        dup(pfds[0]);   /* make stdin same as pfds[0] */
        close(pfds[1]); /* we don't need this */
        char *k = {"wc", "-w", NULL};
        execve("/usr/bin/wc", k, NULL);
    }

    return 0;
}

It looks like there's nothing coming out by running the code, I am not sure what else do I need to make it work.

I am expecting out put of 3, as you will see by entering

echo one two three | wc -w in the terminal. I am using a MacOS by the way.

1) you should not even try to close 1 and 0. 2) your use of dup function and pipe output isn't right. Better read the Manual pages for complete explanations, but the idea is to create a "pipe" that your program will use with pipe (). The parent of the fork will write in 1 side of this pipe, the child will read on the other side. You then need to dup 1 or 0 of the entrance / exit of the pipe (depending). Finally don't forget to close the part of the pipe you are not using (only in the parent / child separated code, do not close both in the main part or nothing will work B-)) — Angevil, Jul 28 '19 at 21:35
Note that you are passing the executed programs an empty environment. That's not what you're supposed to do. Either pass in a custom environment, or use the process's own environment (`extern char **environ;` outside any function — this is not declared in any POSIX header — and then use `environ` as the third argument to `execve()`, or use `execv()` instead of `execve()` since you aren't tinkering with the environment. — Jonathan Leffler, Jul 28 '19 at 23:37
@Angevil the dup usage seems almost right but ugly - should use dup2 instead — Antti Haapala -- Слава Україні, Jul 28 '19 at 23:39
In any case, your compiler should have produced diagnostics messages! If not then time to up warning level! — Antti Haapala -- Слава Україні, Jul 28 '19 at 23:41
@AnttiHaapala You are right about the warning, I could have figured out the problem if I took a look at that. I am not sure what do you mean by "dup was almost right but ugly", could you share some more info? — YiLuo, Jul 28 '19 at 23:47
Well close(1) followed by dup could have changed fd 0 instead if 0 wasn't open. That's why you should always use dup2 if you care about the resulting number — Antti Haapala -- Слава Україні, Jul 28 '19 at 23:48
You aren't closing enough file descriptors in the processes. **Rule of thumb**: If you [`dup2()`](http://pubs.opengroup.org/onlinepubs/9699919799/functions/dup2.html) one end of a pipe to standard input or standard output, close both of the original file descriptors from `pipe()` as soon as possible. In particular, that means before using any of the [`exec*()`](http://pubs.opengroup.org/onlinepubs/9699919799/functions/execvp.html) family of functions. The rule also applies with either `dup()` or [`fcntl()`](http://pubs.opengroup.org/onlinepubs/9699919799/functions/fcntl.html) with `F_DUPFD`. — Jonathan Leffler, Jul 28 '19 at 23:57
@AnttiHaapala Indeed, this code is not robust against that case, but unless the asker is running the program in a strange way, that should not prevent it from working when s/he runs it. — user253751, Jul 29 '19 at 00:07
Programs are entitled to assume that each of standard input, standard output and standard error are open — and on Unix, that means that each of file descriptors 0, 1, and 2 are open. The program works correctly if it is given the environment it is entitled to assume it is given. If it is launched with file descriptors 0 and 1 closed, it would work correctly if `dup2()` was used because `pfds[0]` would contain `0` and `pfds[1]` would contain 1, and `dup2(pfds[0], 0)` would not close `0`, and `dup2(pfds[1], 1)` would not close `1`. Etc. You have to try quite hard to break it with `dup2()`. — Jonathan Leffler, Jul 29 '19 at 00:32
regarding the calculation of `k` (both of them) the `k` should be an array of pointers to strings. So rather than: `char *k = {"echo", "one", "two", "three", NULL};` Suggest: `char *k[] = {"echo", "one", "two", "three", NULL};` Notice that `k` is now an array of pointers to strings — user3629249, Jul 29 '19 at 01:10

Ry- · Accepted Answer · 2019-07-28T22:11:41.227

3

The issue is that you’re assigning an array of strings to a char*. Both ks should be declared char* k[] = …. If your compiler didn’t warn you about this, you need to enable more warnings.

Contrary to the comment, you’re using close and dup correctly (but dup2 would be better).

edited Jul 28 '19 at 22:11

answered Jul 28 '19 at 21:56

Ry-

218,210
55
464
476

The calls to `close()` and `dup()` are, as you say, correct in themselves, and using `dup2()` would also be better. However, as I've noted in a [comment](https://stackoverflow.com/questions/57245103/what-is-the-correct-step-to-execute-execve#comment100992341_57245103), the other dup'd file descriptor should be closed too. This would be noticeable if instead of executing `echo`, the first command read its standard input; it wouldn't terminate because it would not get EOF on its standard input, because it has the write end of the pipe open, even though it'll never write to it. – Jonathan Leffler Jul 28 '19 at 23:59
@JonathanLeffler: Isn’t it closing that descriptor (`dup(pfds[1]); close(pfds[0]);`), and the issue is that there’s an unnecessary descriptor hanging around? Or am I misunderstanding? – Ry- Jul 29 '19 at 00:18
No; the `dup(pfds[1])` does not close `pfds[1]`, and neither does `close(pfds[0])` — so `pfds[1]` (probably file descriptor 4) is still open in the process when the exec occurs. It isn't marked `O_CLOEXEC` (close on exec) so the `exec` doesn't close it either. (No `F_DUPFD_CLOEXEC` or `FD_CLOEXEC` either…) – Jonathan Leffler Jul 29 '19 at 00:21
@JonathanLeffler: “if instead of executing echo, the first command read its standard input; it wouldn't terminate because it would not get EOF on its standard input” – its stdin hasn’t changed, has it? – Ry- Jul 29 '19 at 00:31
2

Hmmm…you're probably correct that my alternative scenario would not create the problem I claimed — but even if I didn't diagnose a valid scenario (I was trying to keep it simple, and missed), nevertheless, in more complex pipelines, not closing both pipe file descriptors can lead to problems. It is best not to leave file descriptors open that should not be open. In some scenarios, it can be a security risk, too. – Jonathan Leffler Jul 29 '19 at 00:35

What is the correct step to execute execve()?

1 Answers1