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);
}
```

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.