perror usage in this case

Question

Can we check that my perror usage is correct? The background is these questions How to handle errors in execvp? How to use perror with dup2? and now I have this code which works but is the code correct?

/* Helper function that forks pipes */
int fork_pipes (int n, struct command *cmd) {
    int i;
    int in, fd [2];
    for (i = 0; i < n - 1; ++i) {
        pipe (fd);
        spawn_proc (in, fd [1], cmd + i);
        close (fd [1]);
        in = fd [0];
    }
    dup2 (in, 0);
    /*return execvp (cmd [i].argv [0], (char * const *)cmd [i].argv);*/
    if (execvp (cmd [i].argv [0], (char * const *)cmd [i].argv) < 0) {
        perror("execvp failed");
        exit(1);
    } else {
        return execvp (cmd [i].argv [0], (char * const *)cmd [i].argv);
    }
}

The full program is

#include <sys/types.h>
#include <errno.h>
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <string.h>
struct command
{
    const char **argv;
};
/* Helper function that spawns processes */
int spawn_proc (int in, int out, struct command *cmd) {
    pid_t pid;
    if ((pid = fork ()) == 0) {
        if (in != 0) {
            /*if (dup2(in, 0) == -1) {
                perror("dup2 failed");
                exit(1);
            }*/
            dup2 (in, 0);
            close (in);
        }
        if (out != 1) {
            dup2 (out, 1);
            close (out);
        }
        if (execvp(cmd->argv [0], (char * const *)cmd->argv) < 0) {
            perror("execvp failed");
            exit(1);
        }
    } else if (pid < 0) {
        perror("fork failed");
        exit(1);
    }
    return pid;
}
/* Helper function that forks pipes */
int fork_pipes (int n, struct command *cmd) {
    int i;
    int in, fd [2];
    for (i = 0; i < n - 1; ++i) {
        pipe (fd);
        spawn_proc (in, fd [1], cmd + i);
        close (fd [1]);
        in = fd [0];
    }
    dup2 (in, 0);
    /*return execvp (cmd [i].argv [0], (char * const *)cmd [i].argv);*/
    if (execvp (cmd [i].argv [0], (char * const *)cmd [i].argv) < 0) {
        perror("execvp failed");
        exit(1);
    } else {
        return execvp (cmd [i].argv [0], (char * const *)cmd [i].argv);
    }
}

int main (int argc, char ** argv) {
    int i;
    if (argc == 1) { /* There were no arguments */
        const char *printenv[] = { "printenv", 0};
        const char *sort[] = { "sort", 0 };
        const char *less[] = { "less", 0 };
        struct command cmd [] = { {printenv}, {sort}, {less} };
        return fork_pipes (3, cmd);
    }
    if (argc > 1) { /* I'd like an argument */

        if (strncmp(argv[1], "cd", 2) && strncmp(argv[1], "exit", 2)) {
            char *tmp;
            int len = 1;
            for( i=1; i<argc; i++)
            {
                len += strlen(argv[i]) + 2;
            }
            tmp = (char*) malloc(len);
            tmp[0] = '\0';
            int pos = 0;
            for( i=1; i<argc; i++)
            {
                pos += sprintf(tmp+pos, "%s%s", (i==1?"":"|"), argv[i]);
            }
            const char *printenv[] = { "printenv", 0};
            const char *grep[] = { "grep", "-E", tmp, NULL};
            const char *sort[] = { "sort", 0 };
            const char *less[] = { "less", 0 };
            struct command cmd [] = { {printenv}, {grep}, {sort}, {less} };
            return fork_pipes (4, cmd);
            free(tmp);
        } else if (! strncmp(argv[1], "cd", 2)) { /* change directory */
            printf("change directory to %s\n" , argv[2]);
            chdir(argv[2]);
        } else if (! strncmp(argv[1], "exit", 2)) { /* change directory */
            printf("exit\n");
            exit(0);
        }
    }
    exit(0);
}

Answer 1

this code:

 if (execvp (cmd [i].argv [0], (char * const *)cmd [i].argv) < 0) {
    perror("execvp failed");
    exit(1);
 } else {
    return execvp (cmd [i].argv [0], (char * const *)cmd [i].argv);
 }

has several problems.

1. execvp does not return, except if an error occurs, so if all works, the enclosing function will never return.

2. the value 'i' is already past the end of the array in 'cmd' due to the prior loop, so 'cmd[i].argv[0] is not correct.

3. cmd is not an array, of struct command, so should not be indexed

4. the first entry in cmd.argv is a pointer to an array where the last entry is NULL. the execvp will work on that (and only that) array so all other pointers to arrays will be ignored