Implementing a pipe-like program without wait?

Question

So I implemented a program that takes an input file, two command strings and an output file to mimick the behaviour of running :

<input cmd1 -option | cmd2 -option > output

that's called like this :

./pipe input "cmd1 -opt" "cmd2 -opt" output

and I did without using the wait system call since while there are open file descriptors on the pipe I opened, the exec'd commands will wait for one another. that is the pipe takes care of coordination IIUC.

But I feel like I am doing it wrong since it seems that people I using wait() out of convention. Is it reasonnable to think it is not necessary in my case since I only need the return value of the second command and the pipe ensures communication or am I missing something ? What else am I doing wrong in terms of code structure or style ?

Here is my code :

#include <unistd.h>
#include <fcntl.h>
#include <stdio.h>
#include <stdlib.h>
#include <errno.h>
#include <string.h>
#include "pipex.h"
static const char g_cmd_not_found[] = {
 "command not found: "
};
static const char g_empty_string[] = {
 "The name of the input or output file cannot be an empty string\n"
};
static int open_or_die(char *filename, int flags, mode_t mode)
{
 int fd;
 fd = open(filename, flags, mode);
 if (fd == -1)
 {
 if (*filename == '0円')
 write(STDERR_FILENO, g_empty_string, sizeof(g_empty_string));
 else
 {
 write(STDERR_FILENO, "pipex: ", sizeof("pipex: "));
 perror(filename);
 }
 exit(EXIT_FAILURE);
 }
 return (fd);
}
static void pipe_or_die(int *pipe_fds)
{
 int r;
 r = pipe(pipe_fds);
 if (r == -1)
 {
 write(STDERR_FILENO, "pipex: ", sizeof("pipex: "));
 perror("pipe");
 exit(EXIT_FAILURE);
 }
}
static void file_is_ok_or_die(char **cmdv, char **pathvar_entries)
{
 if (access(cmdv[0], X_OK) == -1)
 {
 write(STDERR_FILENO, "pipex: ", sizeof("pipex: "));
 if (cmdv[0][0] != '/')
 {
 write(STDERR_FILENO, g_cmd_not_found, sizeof(g_cmd_not_found));
 ft_puts_stderr(cmdv[0]);
 }
 else
 perror(cmdv[0]);
 free_null_terminated_array_of_arrays(cmdv);
 free_null_terminated_array_of_arrays(pathvar_entries);
 if (errno == ENOENT)
 exit(127);
 else if (errno == EACCES)
 exit(126);
 else
 exit(EXIT_FAILURE);
 }
}
void execute_pipeline(char *cmd_str, int read_from, int write_to, char **env)
{
 char **pathvar_entries;
 char **cmdv;
 redirect_fd_to_fd(0, read_from);
 redirect_fd_to_fd(1, write_to);
 pathvar_entries = ft_split(get_path_var(env), ':');
 cmdv = ft_split(cmd_str, ' ');
 if (!pathvar_entries || !cmdv)
 {
 write(STDERR_FILENO, "pipex: ", sizeof("pipex: "));
 perror("malloc");
 free_null_terminated_array_of_arrays(cmdv);
 free_null_terminated_array_of_arrays(pathvar_entries);
 exit(EXIT_FAILURE);
 }
 cmdv[0] = get_command_path(cmdv, get_pwd_var(env), pathvar_entries);
 file_is_ok_or_die(cmdv, pathvar_entries);
 execve(cmdv[0], cmdv, env);
 free_null_terminated_array_of_arrays(cmdv);
 free_null_terminated_array_of_arrays(pathvar_entries);
}
int main(int ac, char **av, char **envp)
{
 int pipefd[2];
 int child_pid;
 int infile_fd;
 int outfile_fd;
 if (ac != 5)
 print_usage_exit();
 pipe_or_die(pipefd);
 child_pid = fork();
 if (child_pid == -1)
 perror("fork");
 else if (child_pid == 0)
 {
 infile_fd = open_or_die(av[1], O_RDONLY, 0000);
 close(pipefd[0]);
 execute_pipeline(av[2], infile_fd, pipefd[1], envp);
 }
 else
 {
 outfile_fd = open_or_die(av[ac - 1], O_WRONLY | O_CREAT | O_TRUNC, 0666);
 close(pipefd[1]);
 execute_pipeline(av[3], pipefd[0], outfile_fd, envp);
 }
 return (EXIT_SUCCESS);
}
```

• Toby Speight: What's "pipex.h"? It seems to be essential for this program, but I don't know what I need to install to compile this.
• cassepipe: @TobySpeight It's just a global header. I haven't put all the functions I am using because it would be too big and some of them are not relevant to what mattered fr me to show you
• Toby Speight: You might like to post it for its own review - I'm wondering what's in redirect_fd_to_fd() that plain old dup2() doesn't do, for instance. And I'd like to know whether ft_split() is any better than passing the command string to sh -c, for a much simpler exec() setup.
• cassepipe: @TobySpeight Thanks for the tip. This is an exercise and I am only allowed to use execve. Program usage is like ./pipex infile "cmd1 -op1 -op2" "cm2 -op1 -op2" outfile which is why I have to use a split, to split the command string. redirect_fd_to_fd doesn't to much than dup2 but for calling close before dup2.

Answer 1

Error handling

The way you handle error is quite unusual. Why use write() instead of fprintf(stderr, ...)? Why have some error messages stored in a variable like g_empty_string, but other errors messages are passes as literals, like "pipex: "? Why try to open() first and only if it fails check if filename is empty?

On Linux, I recommend you use err() to report errors and exit with an error code in one go. For example:

static int open_or_die(const char *filename, int flags, mode_t mode)
{
 if (!filename[0])
 err(EXIT_FAILURE, "Empty filename");
 int fd = open(filename, flags, mode);
 if (fd == -1)
 err(EXIT_FAILURE, "Error trying to open '%s'", filename);
 return fd;
}

Don't use access() to check if you can execve()

Instead of first checking with access() if a file is executable before calling execve(), just call execve() unconditionally, and then just check the return value of execve(). Otherwise, you will have a TOCTTOU bug.

if (execve(cmdv[0], cmdv, env) == -1)
 err(EXIT_FAILURE, "Could not execute '%s'", cmdv[0]);

Note that if execve() succeeds, it will never return, so there's no need to free anything afterwards.

Why you should wait()

If you don't call wait(), your program will terminate when the second command terminates. However, consider that the first command might still be doing something. It will then continue in the background, but you won't have control over it anymore. Suppose you want to call ./pipe twice, and the second call depends on results from the first call, then you would really want to ensure both processes of the first call have finished.

• jdt: I would perhaps suggest that the OP use pthreads or openMP to create threads and join them instead of waiting and hoping that the work is done?
• G. Sliepen: @upkajdt Unfortunately execve() replaces the whole process, not just one thread. Also see this StackOverflow question.
• cassepipe: @G.Sliepen You say "It will then continue in the background, but you won't have control over it anymore." Indeed I can see that ./pipex /dev/stdin cat ls outfile does not wait for input indeed but I am not sure why it is the case. Why can't the zombie cat read from stdin even though the main process is over ? It should still run right ?
• G. Sliepen: @cassepipe cat will still run but then the question is: what happens with /dev/stdin after ls exits? If I try to run that command in a shell, then after it finishes, the cat process is still running, but as soon as I press any key it exits with the error: cat: -: Input/output error.
• cassepipe: @G.Sliepen Not sure I understand but to answer your question I have no idea as to what happens to /dev/stdin after ls exits. I'd say nothing since I opened the input file in the child process. When I run </dev/stdin cat | ls in a terminal it prints the ls output and then waits for input. I can enter one line, press Return and then I get back to the shell prompt, no error message.