Two Pipes to send a string between a parent and child process

I need to make a child process and create two pipes each going one way between the parent and child process. I have to send a message ("Greetings") to the child byte by byte which reads it byte by byte and then converts the chars to uppercase and sends the message back to the parent in the same fashion. I am having trouble with the pipe aspect of it, my program works with one pipe and then I tried to add a pipe and got this.

Code:

---


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

#define BUFFER_SIZE 25
#define READ_END 0
#define WRITE_END 1
#define READ_END2 0
#define WRITE_END2 1

int main(void)
{
  int ch = 0;
  int ch2 = 0;
  char write_msg[BUFFER_SIZE] = "Greetings";
  char write_msg2[BUFFER_SIZE];
  char read_msg[BUFFER_SIZE];
  int fd[2];
  int fd2[2];
  pid_t pid;
  if (pipe(fd) == -1) {
    fprintf(stderr, "Pipe failed");
    return 1;
  }
  if (pipe(fd2) == -1) {
    fprintf(stderr, "Pipe failed");
    return 1;
  }
  pid = fork();

  if (pid < 0) {
    fprintf(stderr, "Fork failed");
    return 1;
  }

  if (pid>0){
    close(fd[READ_END]);
    close(fd2[WRITE_END2]);
   
    while (write_msg[ch] != 0)
    {
      write(fd[WRITE_END], &write_msg[ch], strlen(write_msg)+1);
      ch++;
    }
    close(fd[WRITE_END]);
    while (read_msg[ch] != 0)
    {
      read(fd2[READ_END2], &read_msg[ch2], 1);
      printf("%c", read_msg[ch2]);
      if (ch2%2 == 0) {
    ch2++;
      }
    }
  }

  else {
    close(fd[WRITE_END]);
    close(fd2[READ_END2]);
    ch = 0;
    ch2 =0;
    while (read_msg[ch] != 0)
    {
      read(fd[READ_END], &read_msg[ch], 1);
      write_msg2[ch] = toupper(&read_msg[ch]);
      printf("%c", read_msg[ch]);
      if (ch%2 == 0) {
    ch++;
      }
    }
    while (write_msg2[ch2] != 0)
    {
      write(fd2[WRITE_END2], &write_msg2[ch2], strlen(write_msg2)+1);
      ch2++;
    }

    close(fd2[READ_END]);
  }
  return 0;
}

---

> int fd[2];
> int fd2[2];
Consider more meaningful variable names like say parentToChildPipe and childToParentPipe

> write(fd[WRITE_END], &write_msg[ch], strlen(write_msg)+1);
You only want to write one character at a time.

Or you just forego the loop altogether and just write the whole string to the pipe in one hit.

Turn up the warnings on your compiler and make sure you fix them all:

Code:

---

$ gcc -Wall -g -std=c99  pipe.c  -o pipe
pipe.c: In function ‘main’:
pipe.c:66: warning: implicit declaration of function ‘toupper’

---

You need to #include <ctype.h> to use toupper(). Without the prototype, C assumes it's declared int toupper(), meaning it takes any number and type of parameters, and returns an int. Including ctype, you can see you're using it incorrectly:

Code:

---

$ gcc -Wall -g -std=c99  pipe.c  -o pipe
pipe.c: In function ‘main’:
pipe.c:67: warning: passing argument 1 of ‘toupper’ makes integer from pointer without a cast
/usr/include/ctype.h:119: note: expected ‘int’ but argument is of type ‘char *’

---

toupper works on individual chars (as ints), not on strings or pointers. Drop the & from line 67 (66 before adding the missing include).


Other issues:

• READ_END2 and WRITE_END2 are redundant, ditch them.
• You don't need ch and ch2 since you're never using them simultaneously. Just use ch and reset it to zero before using it again.
• As it stands, your while loop on lines 48-55 is using both ch and ch2, which is almost certainly incorrect.
• I'm not sure why you have those if statements that check ch%2 == 0, they seem wrong, but your problem is poorly specified. Is there an "every other character" component? If not, ditch this.
• write_msg2 and read_msg are uninitialized. Your while loops that start with while (read_msg[ch] != 0) and while (write_msg2[ch] != 0) may never actually execute because of this (if by chance the first byte is a null character).
• You never check the return value of your calls to read or write, so you don't know whether they worked and what, if any problem there was.
• When you don't read/write byte by byte, read() and write() may not write the full amount of data you asked it to, thus you would need better logic to detect this. Again, check the return value of read() and write().

For debugging purposes, put lots of print statements in there. Here's my version of the first loop, where the parent sends the original string to the child. All four of your loops should be this thorough, and it's easier if you read/write one byte at a time in all of them.

Code:

---

ch = 0;
do {
    rv = write(parent_to_child[WRITE_END], &write_msg[ch], 1);
    if (rv == -1) {
        perror("Error writing in parent");
    }
    else if (rv == 0 && write_msg[ch] != '\0') {
        fprintf(stderr, "Parent couldn't write full string, perhaps the child died or closed the connection\n");
    }
    else {
        printf("Parent wrote '%c'\n", write_msg[ch]);
    }
} while (rv > 0 && write_msg[ch++] != 0);

---