I'm struggling to implement a shell with pipelines for class.
typedef struct {
char** cmd;
int in[2];
int out[2];
} cmdio;
cmdio cmds[MAX_PIPE + 1];
Commands in the pipeline are read and stored in cmds
.
cmdio[i].in
is the pair of file descriptors of the input pipe returned by pipe()
. For the first command, which reads from terminal input, it is just {fileno(stdin), -1}. cmdin[i].out
is similar for the output pipe/terminal output. cmdio[i].in
is the same as cmd[i-1].out
. For example:
$ ls -l | sort | wc
CMD: ls -l
IN: 0 -1
OUT: 3 4
CMD: sort
IN: 3 4
OUT: 5 6
CMD: wc
IN: 5 6
OUT: -1 1
We pass each command to process_command, which does a number of things:
for (cmdi = 0; cmds[cmdi].cmd != NULL; cmdi++) {
process_command(&cmds[cmdi]);
}
Now, inside process_command:
if (!(pid_fork = fork())) {
dup2(cmd->in[0], fileno(stdin));
dup2(cmd->out[1], fileno(stdout));
if (cmd->in[1] >= 0) {
if (close(cmd->in[1])) {
perror(NULL);
}
}
if (cmd->out[0] >= 0) {
if (close(cmd->out[0])) {
perror(NULL);
}
}
execvp(cmd->cmd[0], cmd->cmd);
exit(-1);
}
The problem is that reading from the pipe blocks forever:
COMMAND $ ls | wc
Created pipe, in: 5 out: 6
Foreground pid: 9042, command: ls, Exited, info: 0
[blocked running read() within wc]
If, instead of exchanging the process with execvp
, I just do this:
if (!(pid_fork = fork())) {
dup2(cmd->in[0], fileno(stdin));
dup2(cmd->out[1], fileno(stdout));
if (cmd->in[1] >= 0) {
if (close(cmd->in[1])) {
perror(NULL);
}
}
if (cmd->out[0] >= 0) {
if (close(cmd->out[0])) {
perror(NULL);
}
}
char buf[6];
read(fileno(stdin), buf, 5);
buf[5] = '\0';
printf("%s\n", buf);
exit(0);
}
It happens to work:
COMMAND $ cmd1 | cmd2 | cmd3 | cmd4 | cmd5
Pipe creada, in: 11 out: 12
Pipe creada, in: 13 out: 14
Pipe creada, in: 15 out: 16
Pipe creada, in: 17 out: 18
hola!
Foreground pid: 9251, command: cmd1, Exited, info: 0
Foreground pid: 9252, command: cmd2, Exited, info: 0
Foreground pid: 9253, command: cmd3, Exited, info: 0
Foreground pid: 9254, command: cmd4, Exited, info: 0
hola!
Foreground pid: 9255, command: cmd5, Exited, info: 0
What could be the problem?
You haven't got enough closes. In the code:
if (!(pid_fork = fork())) {
dup2(cmd->in[0], fileno(stdin));
dup2(cmd->out[1], fileno(stdout));
if (cmd->in[1] >= 0) {
if (close(cmd->in[1])) {
perror(NULL);
}
}
if (cmd->out[0] >= 0) {
if (close(cmd->out[0])) {
perror(NULL);
}
}
execvp(cmd->cmd[0], cmd->cmd);
exit(-1);
}
After you've duplicated the pipes to stdin
and stdout
(via fileno()
), you need to close the pipes:
if (!(pid_fork = fork())) {
dup2(cmd->in[0], fileno(stdin));
dup2(cmd->out[1], fileno(stdout));
if (cmd->in[1] >= 0) {
if (close(cmd->in[1])) {
perror(NULL);
}
}
if (cmd->out[0] >= 0) {
if (close(cmd->out[0])) {
perror(NULL);
}
}
close(cmd->in[0]); // Or your error checked version, but I'd use a function
close(cmd->out[1]);
execvp(cmd->cmd[0], cmd->cmd);
exit(-1);
}
The programs aren't finishing because there is a write end of the file still open. Also, don't forget that if the parent process (shell) creates the pipes, it must close both ends of the pipe. Not closing enough pipes is probably the most common mistake when starting to learn plumbing with pipes.
All right, I finally solved it.
On the parent process, just after the whole child fork, I put:
f (cmd->in[0] != fileno(stdin)) {
close(cmd->in[0]);
close(cmd->in[1]);
}
And voilà. I had done something like that before but I mistyped and did close(cmd->out[0]);
instead. So that's it. It makes sense now.
If you love us? You can donate to us via Paypal or buy me a coffee so we can maintain and grow! Thank you!
Donate Us With