Program that read file and send it to parent proce

Question

I need to write a program that create pipe send filename from command line to child process. In child read that file and send it back using pipe. Parent process should print the file. if error occur in child process error must be send to parent process.

here is my code, it print some junk along file file (and also it disable scrolling in terminal emulator when I run it).

#include <stdio.h>
#include <unistd.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>

void main(int argc, char *argv[]) {
   int pipefd[2];
   char buff[100];
   int childpid;
   int size;
   FILE *file;

   if (argc != 2) {
      printf("usage:\n%s <filename>\n", argv[0]);
      exit(1);
   }
   if (pipe(pipefd) < 0) {
       perror("can't open pipe\n");
   }
   if ((childpid = fork()) == 0) {
      sleep(1);
      size = read(pipefd[0], buff, sizeof(buff));
      file = fopen(buff, "r");
      if (file == NULL) {
         write(pipefd[1], "Can't open file", 15);
         exit(1);
      }
      while (!feof(file)) {
         if (fgets(buff, sizeof(buff), file) == NULL) {
            write(pipefd[1], "Error reading file", 18);
         } else {
            write(pipefd[1], buff, sizeof(buff));
         }
      }
   } else if (childpid > 0) {
      size = strlen(argv[1]);
      if (write(pipefd[1], argv[1], size) != size) {
         perror("Error writing to pipe\n");
      }
      wait(NULL);
      while ((size = read(pipefd[0], buff, sizeof(buff))) > 0) {
         write(1, buff, size);
      }
   }
   exit(0);
}

Answer 1

Your program works as intended after quite a few changes. Lets list out what all changes are required and why-

I) Both in the child and parent, close the respective pipes as soon as you are done with them. From man page of read(3),

If some process has the pipe open for writing and O_NONBLOCK is clear, read() shall block the calling thread until some data is written or the pipe is closed by all processes that had the pipe open for writing.

So do something like this in your code everywhere where the job pipes is over,

  size = read(pipefd[0], buff, sizeof(buff));
  close(pipefd[0]);

  write(pipefd[1], buff, strlen(buff));
  close(pipefd[1]);

  if (write(pipefd[1], argv[1], size) != size) {
     perror("Error writing to pipe\n");
  }
  close(pipefd[1]);

  while ((size = read(pipefd[0], buff, sizeof(buff))) > 0) 
  {
     write(1, buff, size);
  }
  close(pipefd[0]);

You hadn't closed the write end of of the pipe in the child and your parent was blocking in the read

II) You are using something like while(fgets(...)) in a loop to read data from file. This will bomb when there are newlines in the file and fgets returns multiple times, overwriting the buffer everytime during the process

I always use simple fgetc and feof combination to read from file. So, change your file reading mechanism to something like

unsigned count=0;
while (!feof(file) && count < sizeof(buff))
    buff[count++]=fgetc(file);
if (feof(file)) 
    buff[--count]=0;
else
    buff[sizeof(buff)-1]=0;

III) While writing the file data from the child, you should use strlen(as we have already made sure buffer is null terminated, see above ) and not sizeof as the buffer may not be full at all and you will end up writing junk. So, change

  write(pipefd[1], buff, sizeof(buff));

to

  write(pipefd[1], buff, strlen(buff));

IV) Follow a safe exit from the child and parent after their job is done. Something like

close(pipefd[1]);
_exit(EXIT_SUCCESS);   // in child

and

close(pipefd[0]);
exit(EXIT_SUCCESS); // in parent

PS: I've changed the file reading logic, so your compiler error is gone now and do follow the advice given by n.m.

Answer 2

This code does not compile:

  while (fgets(buff, sizeof(buff), file) != NULL) {
        write(pipefd[1], "Error reading file", 18);
     } else {
        write(pipefd[1], buff, sizeof(buff));
     }

You can't have an else clause there.

Answer 3

You cannot write sizeof(buf) meaningful bytes if fgets returned less than that. The rest will be filled with junk.

Moreover, mixing string-oriented fgets with binary read/write is a bad style. Use read or fread to read the file. They return number of bytes read, use this number as an argument to write.