Bad file descriptor error while implementing pipeline in C
I am trying to implement a sample shell, such as a program that executes the ls | wc
Using pipes to implement the command. When I execute the command, I get the following error.
wc: standard input: invalid file descriptor 0 0 0 wc: -: Bad file descriptor
Please take a look at the code and enter the data. Note: 1) parse is a library that takes the input and returns each command as a linked list with arguments and required data. Parse works fine 2) I am executing each command in a different subprocess, so fork
#include <stdio.h>
#include <stdlib.h>
#include "parse.h"
int pip[3][2];
int main(int argc, char *argv[], char *envp[])
{
Pipe p;
Cmd c;
pipe(pip[0]);
pipe(pip[1]);
pid_t pid;
pid=fork();
char *host = "armadillo";
printf("%s%% ", host);
p = parse();
c=p->head;
printf("1 \n");
pid=fork();
if(pid==0)
{
close(pip[0][0]);
close(STDOUT_FILENO);
dup2(pip[0][1],STDOUT_FILENO);
execvp(c->args[0],c->args);
}
else
{
waitpid(pid,NULL,0);
}
printf("2 \n");
close(pip[0][1]);
close(pip[0][0]);
c=c->next;
printf("%s \n",c->args[0]);
pid=fork();
if(pid==0)
{
close(STDIN_FILENO);
dup2(pip[0][0],STDIN_FILENO);
close(pip[0][1]);
execvp(c->args[0],c->args);
}
else
{
waitpid(pid,NULL,0);
close(pip[0][1]);
close(pip[0][0]);
}
}
source to share
I took the lazy exit and wrote my own rather than fixing other code. Treat this as "just another example of connecting pipes in C", but it might help point out problems with the OP's code.
/*
* hard-wired example program exploring how to implement
*
* system("ls | wc");
*
* using calls to pipe(2), fork(2), execvp(2) and wait(2)
*/
#include <sys/types.h>
#include <sys/wait.h>
#include <errno.h>
#include <unistd.h>
#include <stdio.h>
#include <stdlib.h>
static void
do_close(int fd)
{
if (close(fd) == -1) {
perror("close");
exit(1);
}
}
static void
do_execvp(char *const cmd[])
{
execvp(cmd[0], cmd);
/*
* if execvp returns in this text, an error occured.
*/
perror("execvp");
exit(1);
}
static void
dup_and_exec(int fd, int *pp, char *const cmd[])
{
if (dup2(pp[fd], fd) == -1) {
perror("dup2");
exit(1);
}
do_close(pp[0]);
do_close(pp[1]);
do_execvp(cmd);
}
int
main(void)
{
char *const ls_cmd[] = { "ls", 0 };
char *const wc_cmd[] = { "wc", 0 };
int fds[2];
int w_stat;
pid_t ls_pid, wc_pid, w_pid;
/* create a single pipe to connect our writer and reader processes */
if (pipe(fds) == -1) {
perror("pipe");
exit(1);
}
/* create the writer process: ls */
ls_pid = fork();
if (ls_pid == -1) {
perror("fork");
exit(1);
}
if (ls_pid == 0) {
/* this is the child - do the "ls" command */
dup_and_exec(1, fds, ls_cmd); /* no return from here */
}
/* create the reader process: wc */
wc_pid = fork();
if (wc_pid == -1) {
perror("fork");
exit(1);
}
if (wc_pid == 0) {
/* this is the child - do the "wc" command */
dup_and_exec(0, fds, wc_cmd); /* no return from here */
}
/* parent process */
/*
* It important to close the pipe completely in the parent,
* so (in particular) there no process that could be an
* additional writer to the "write" side of the pipe.
*
* We need to arrange things so that our reader process (the "wc"
* process in this example) will see EOF when the only writer (the
* "ls" process) closes its output and exits.
*
* If this parent process does not close the write side of the pipe,
* it remains open, since it shared across fork(2), so the reader
* (wc) won't ever see EOF and exit, and this parent process won't
* ever see the wc exit, and everything hangs.
*
* The core problems will have started with the parent, which all
* children know to be true.
*
* The next lines also close the "read" side of the pipe, which
* is a bit cleaner, but won't affect proper operation of this
* sample program. But closing all un-needed file descriptors is
* good hygiene: for longer running applications, or for library
* code that could be called from longer running programs, avoiding
* any leaks of file descriptors is a good thing.
*/
do_close(fds[0]);
do_close(fds[1]);
while ((w_pid = wait(&w_stat)) > 0) {
printf("%s process exited", w_pid == ls_pid ? "ls" : "wc");
if (WIFEXITED(w_stat)) {
printf(" (status %d)", WEXITSTATUS(w_stat));
}
fputs("\n", stdout);
}
if (w_pid == -1 && errno != ECHILD) {
perror("wait");
exit(1);
}
return 0;
}
source to share
There are several problems with the code:
You make the grandchild of the initial process
pid=fork();
char *host = "armadillo";
printf("%s%% ", host);
p = parse();
c=p->head;
printf("1 \n");
pid=fork(); // this fork here is wrong
You unfold and then unfold again for the parent to make the child and then both of them make the child one at a time. At this point, you already have 4 processes .
This part of the code will have something like this:
pid_t pid;
pid=fork();
char *host = "armadillo";
printf("%s%% ", host);
p = parse();
c=p->head;
printf("1 \n");
// pid=fork(); // it'll be in another part
if (pid == -1) {
// print error
exit(1);
} else if (pid == 0) {
//child
close(pip[0][0]);
close(STDOUT_FILENO);
dup2(pip[0][1],STDOUT_FILENO);
close(pip[0][1]); // I added this
execvp(c->args[0],c->args);
}
//parent
waitpid(pid,NULL,0); // it not a good idea but I leave it here
printf("2 \n");
// now you can fork again and use the same pid variable
pid=fork();
The child is expected to finish.
if(pid==0)
{
close(pip[0][0]);
close(STDOUT_FILENO);
dup2(pip[0][1],STDOUT_FILENO);
execvp(c->args[0],c->args);
}
else
{
waitpid(pid,NULL,0); // you have more commands to execute yet, so you must do it before this
}
Waitpid is not needed at all if you are using the parent process to execute the last command on the pipe ( wc
). But it is up to you if you want to have a parent process. If so, you should call waitpid
when all the children have completed their tasks.
You must not close the pipe until dup2 .. Because of this, you noticed an error.
wc: standard input: Bad file descriptor 0 0 0 wc: -: Bad file descriptor
After dup2, you have to close the pipe in the child.
close(pip[0][0]); // it ok
close(STDOUT_FILENO); // it ok but not necessary
dup2(pip[0][1],STDOUT_FILENO);
// here you have to close(pip[0][1]) due to you have already duped it in STDOUT_FILENO
execvp(c->args[0],c->args);
If you have a parent, you only have to close it once when both children cheated on it.
printf("2 \n");
close(pip[0][1]);
close(pip[0][0]); // You're closing the file descriptor which wc needs to read.
You do not check all possible return status of some functions.
pipe
fork
execvp
dup2
Is there something else to improve
int pip[3][2]; // in your case with `int pip[2]` would be enough
pipe(pip[0]);
pipe(pip[1]); // in your case you have to create just one pipe
source to share