General remarks
Reiterating what I've said before on SO in other answers.
You aren't closing enough file descriptors in the child process.
Rule of thumb: If you
dup2()
one end of a pipe to standard input or standard output, close both of the
original file descriptors returned by
pipe()
as soon as possible.
In particular, you should close them before using any of the
exec*()
family of functions.
The rule also applies if you duplicate the descriptors with either
dup()
or
fcntl()
with F_DUPFD or F_DUPFD_CLOEXEC.
If the parent process will not communicate with any of its children via
the pipe, it must ensure that it closes both ends of the pipe early
enough (before waiting, for example) so that its children can receive
EOF indications on read (or get SIGPIPE signals or write errors on
write), rather than blocking indefinitely.
Even if the parent uses the pipe without using dup2(), it should
normally close at least one end of the pipe — it is extremely rare for
a program to read and write on both ends of a single pipe.
Note that the O_CLOEXEC option to
open(),
and the FD_CLOEXEC and F_DUPFD_CLOEXEC options to fcntl() can also factor
into this discussion.
If you use
posix_spawn()
and its extensive family of support functions (21 functions in total),
you will need to review how to close file descriptors in the spawned process
(posix_spawn_file_actions_addclose(),
etc.).
Note that using dup2(a, b) is safer than using close(b); dup(a);
for a variety of reasons.
One is that if you want to force the file descriptor to a larger than
usual number, dup2() is the only sensible way to do that.
Another is that if a is the same as b (e.g. both 0), then dup2()
handles it correctly (it doesn't close b before duplicating a)
whereas the separate close() and dup() fails horribly.
This is an unlikely, but not impossible, circumstance.
Analysis of code
The question contains the code:
#include <stdio.h>
#include <unistd.h>
void main(int argv, char *argc) {
int stdin_copy = dup(STDIN_FILENO);
int stdout_copy = dup(STDOUT_FILENO);
int testpipe[2];
pipe(testpipe);
int PID = fork();
if (PID == 0) {
dup2(testpipe[0], 0);
close(testpipe[1]);
execl("./multby", "multby", "3", NULL);// Just multiplies argument 3 with the number in testpipe.
} else {
dup2(testpipe[1], 1);
close(testpipe[0]);
printf("5");
fclose(stdout);
close(testpipe[1]);
char initialval[100];
read(testpipe[0], initialval, 100);
fprintf(stderr, "initial value: %s\n", initialval);
wait(NULL);
dup2(stdin_copy, 0);
dup2(stdout_copy, 1);
printf("done");//was not displayed when I run code.
}
}
The line void main(int argv, char *argc) { should be int main(void) since you do not use the command line arguments. You also have the names argv and argc reversed from the normal convention — the first argument is normally called argc (argument count) and the second is normally called argv (argument vector). Additionally, the type for the second argument should be char **argv (or char **argc if you want to confuse all your casual readers). See also What should main() return in C and C++?
The next block of code that warrants discussion is:
if (PID == 0) {
dup2(testpipe[0], 0);
close(testpipe[1]);
execl("./multby", "multby", "3", NULL);// Just multiplies argument 3 with the number in testpipe.
}
This breaks the rule of thumb. You should also put error handling code after the execl().
if (PID == 0)
{
dup2(testpipe[0], STDIN_FILENO);
close(testpipe[0]);
close(testpipe[1]);
execl("./multby", "multby", "3", (char *)NULL);
fprintf(stderr, "failed to execute ./multby\n");
exit(EXIT_FAILURE);
}
The next block of code to analyze is:
dup2(testpipe[1], 1);
close(testpipe[0]);
printf("5");
fclose(stdout);
close(testpipe[1]);
In theory, you should use STDOUT_FILENO instead of 1, but I have considerable sympathy with the use of 1 (not least because when I first learned C, there was no such symbolic constant). You do actually close both ends of the pipe, but I'd prefer to see both closes immediately after the dup2() call, in line with the rule of thumb. The printf() without a newline does send anything down the pipe; it stashes the 5 in the I/O buffer.
As Chris Dodd identified in their answer, the fclose(stdout) call is a source of much trouble. You should probably simply replace it with fflush(stdout).
Moving on:
char initialval[100];
read(testpipe[0], initialval, 100);
fprintf(stderr, "initial value: %s\n", initialval);
wait(NULL);
dup2(stdin_copy, 0);
dup2(stdout_copy, 1);
printf("done");
You didn't check whether the read() worked. It didn't; it failed with EBADF, because just above this you use close(testpipe[0]);. What you print on stderr there is an uninitialized string — that's not good. In practice, if you want to read information from the child reliably, you need two pipes, one for parent-to-child communication and the other for child-to-parent communication. Otherwise, there's no guarantee that the parent won't read what it wrote. If you waited for the child to die before reading, you'd be in with a decent chance of it working, but you can't always rely on being able to do that.
The first of the two dup2() calls is pointless; you didn't change the redirection for standard input (so in fact stdin_copy is unnecessary). The second changes the assignment of the standard output file descriptor, but you had already closed stdout, so there is no easy way to reopen it so that the printf() would work. The message should end with a newline — most printf() format strings should end with a newline, unless it is deliberately being used to build up a single line of output piecemeal. However, if you paid attention to the return value, you'd find that it failed (-1) and the chances are good that you'd find errno == EBADF again.
Fixing the code — fragile solution
Given this code for multby.c:
#include <stdio.h>
#include <stdlib.h>
int main(int argc, char **argv)
{
if (argc != 2)
{
fprintf(stderr, "Usage; %s number", argv[0]);
return 1;
}
int multiplier = atoi(argv[1]);
int number;
if (scanf("%d", &number) == 1)
printf("%d\n", number * multiplier);
else
fprintf(stderr, "%s: failed to read a number from standard input\n", argv[0]);
return 0;
}
and this code (as pipe23.c, compiled to pipe23):
#include <errno.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
int main(void)
{
int stdout_copy = dup(STDOUT_FILENO);
int testpipe[2];
pipe(testpipe);
int PID = fork();
if (PID == 0)
{
dup2(testpipe[0], 0);
dup2(testpipe[1], 1);
close(testpipe[1]);
close(testpipe[0]);
execl("./multby", "multby", "3", NULL);// Just multiplies argument 3 with the number in testpipe.
fprintf(stderr, "failed to execute ./multby (%d: %s)\n", errno, strerror(errno));
exit(EXIT_FAILURE);
}
else
{
dup2(testpipe[1], 1);
close(testpipe[1]);
printf("5\n");
fflush(stdout);
close(1);
wait(NULL);
char initialval[100];
read(testpipe[0], initialval, 100);
close(testpipe[0]);
fprintf(stderr, "initial value: [%s]\n", initialval);
dup2(stdout_copy, 1);
printf("done\n");
}
}
the combination barely works — it is not a resilient solution. For example, I added the newline after the 5. The child waits for another character after the 5 to determine that it has finished reading the number. It doesn't get EOF because it has the write end of the pipe open for sending the response to the parent, even if it is hung reading from the pipe so it never will write to it. But because it only attempts to read one number, it is OK.
The output is:
initial value: [15
]
done
Fixing the code — robust solution
If you were dealing with arbitrary quantities of numbers, you'd need to use two pipes — it is the only reliable way of doing the task. This would also work for a single number passed to the child, of course.
Here's a modified multby.c which loops on reading:
#include <stdio.h>
#include <stdlib.h>
int main(int argc, char **argv)
{
if (argc != 2)
{
fprintf(stderr, "Usage; %s number", argv[0]);
return 1;
}
int multiplier = atoi(argv[1]);
int number;
while (scanf("%d", &number) == 1)
printf("%d\n", number * multiplier);
return 0;
}
and here's a modified pipe23.c that uses two pipes and writes 3 numbers to the child, and gets back three results. Note that it doesn't need to put a newline after the third number with this organization (though there'd be no harm done if it did include a newline). Also, if you're devious, the second space in the list of numbers is unnecessary too; the - isn't part of the second number, so the scanning stops after the 0.
#include <errno.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
int main(void)
{
int stdout_copy = dup(STDOUT_FILENO);
int p_to_c[2];
int c_to_p[2];
if (pipe(p_to_c) != 0 || pipe(c_to_p) != 0)
{
fprintf(stderr, "failed to open a pipe (%d: %s)\n", errno, strerror(errno));
exit(EXIT_FAILURE);
}
int PID = fork();
if (PID == 0)
{
dup2(p_to_c[0], 0);
dup2(c_to_p[1], 1);
close(c_to_p[0]);
close(c_to_p[1]);
close(p_to_c[0]);
close(p_to_c[1]);
execl("./multby", "multby", "3", NULL);// Just multiplies argument 3 with the number in testpipe.
fprintf(stderr, "failed to execute ./multby (%d: %s)\n", errno, strerror(errno));
exit(EXIT_FAILURE);
}
else
{
dup2(p_to_c[1], 1);
close(p_to_c[1]);
close(p_to_c[0]);
close(c_to_p[1]);
printf("5 10 -15");
fflush(stdout);
close(1);
char initialval[100];
int n = read(c_to_p[0], initialval, 100);
if (n < 0)
{
fprintf(stderr, "failed to read from the child (%d: %s)\n", errno, strerror(errno));
exit(EXIT_FAILURE);
}
close(c_to_p[0]);
wait(NULL);
fprintf(stderr, "initial value: [%.*s]\n", n, initialval);
dup2(stdout_copy, 1);
printf("done\n");
}
}
Note that there are lots of calls to close() in there — two for each of the 4 descriptors involved in handling two pipes. This is normal. Not taking care to close the file descriptors can easily lead to hung systems.
The output from running this pipe23 is this, which is what I wanted:
initial value: [15
30
-45
]
done
