1

I'm writing my minishell and I can't understand why execve doesn't work when calling pid_2 ?
My main task is to implement env | grep LANG


int main(void)
{
    pid_t pid_1, pid_2;
    int fd[2];
    int status;
    char *mass_1[] = {"env", NULL};
    char *mass_2[] = {"grep", "LANG", NULL};

    pid_1 = fork();
    pipe(fd);
    if (pid_1 == 0)
    {
        dup2(fd[1], 0);
        close(fd[0]);
        execve(mass_1[0], mass_1, NULL);
        exit(1);
    }
    pid_2 = fork();
    if (pid_2 == 0)
    {
        dup2(fd[0], 0);
        close(fd[1]);
        execve(mass_2[0], mass_2, NULL);
        exit(1);
    }
    close(fd[0]);
    close(fd[1]);
    waitpid(pid_1, &status, WUNTRACED);
    waitpid(pid_2, &status, WUNTRACED);
    return (0);
}
Mark
  • 27
  • 1
  • 5
  • 1
    `dup2(fd[1], 0);` `dup2(fd[0], 0);` Something wrong, you are `dup2` with `0` twice. Shouldn't one of them connect with stdout with `1`? – KamilCuk Aug 11 '20 at 07:02
  • `pipe()` after `fork()` is wrong too. That creates and uses a separate pipe in the first child instead of the one created by the parent – Ingo Leonhardt Aug 11 '20 at 08:19
  • 1
    `env | grep LANG` will also print variables that contain LANG in the value. There are better ways to do that. See [Get list of variables whose name matches a certain pattern](https://stackoverflow.com/q/511694/995714) – phuclv Aug 11 '20 at 08:51
  • 1
    There's no need to fork and run child processes to access **the environment variables of your own process**. If you're on a POSIX platform, direct access to the entire list of environment variables is done through the `extern char **environ` variable. See https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap08.html On Windows, the environment variables are directly available via [the `GetEnvironmentStrings()` function](https://learn.microsoft.com/en-us/windows/win32/api/processenv/nf-processenv-getenvironmentstrings?redirectedfrom=MSDN) – Andrew Henle Aug 11 '20 at 09:26

1 Answers1

0

There are three issues in your code:

  1. execve() doesn't search for the command in $PATH so generally it won't find neither env nor grep. Use execvp() instead.
  2. You should call pipe() before the first fork() to prevent another pipe to be created and used by the child process
  3. as @KamilCuk has pointed out: in the first child process, you should dup2() stdout instead of stdin

Fixing these bugs lead to this working code

#include <stdlib.h>
#include <sys/types.h>
#include <unistd.h>
#include <sys/wait.h>
#include <stdio.h>
#include <errno.h>

int main(void)
{
    pid_t pid_1, pid_2;
    int fd[2];
    int status;
    char *mass_1[] = {"env", NULL};
    char *mass_2[] = {"grep", "LANG", NULL};

    pipe(fd);
    pid_1 = fork();
    if (pid_1 == 0)
    {
        dup2(fd[1], 1);
        close(fd[0]);
        execvp(mass_1[0], mass_1);
        exit(1);
    }
    pid_2 = fork();
    if (pid_2 == 0)
    {
        dup2(fd[0], 0);
        close(fd[1]);
        execvp(mass_2[0], mass_2);
        exit(1);
    }
    close(fd[0]);
    close(fd[1]);
    waitpid(pid_1, &status, WUNTRACED);
    waitpid(pid_2, &status, WUNTRACED);
    return (0);
}

Btw. you should add some error handling e.g. pipe() or fork() could fail

If you must not call execvp() you have to provide pathnames to execve() like "/usr/bin/env" or "/bin/grep". In this case you also have to provide the parent's environment at least to env otherwise it won't print anything. The easiest way to do so is:

  • change the signature of main() to int main(int argc, char *argv[], char *envp[])
  • call execve(mass_1[0], mass_1, envp);
Ingo Leonhardt
  • 9,435
  • 2
  • 24
  • 33
  • unfortunately this code is not suitable for me as I only need to use execve but thank you for fixing my errors – Mark Aug 11 '20 at 09:02
  • Then you have to find the pathnames of the commands by yourself – Ingo Leonhardt Aug 11 '20 at 09:05
  • If I specify the paths /bin/env and /bin/grep should work ? it works with execvp but it doesn't work with execve although I have specified the paths to the commands – Mark Aug 11 '20 at 09:09
  • Using execve() leads to another problem indeed. Passing NULL as environments leads to `env` not knowing any variables and thus not printing anything. I'm making another edit – Ingo Leonhardt Aug 11 '20 at 09:14
  • I just added int main(int ac, char * * av, char **env) and add env instead of NULL – Mark Aug 11 '20 at 09:18