tags:

views:

142

answers:

1

I am trying to write a program that The parent process will take the arguments to main() and send the characters in them one at a time to the child process through a pipe (one call to write for each character). The child process will count the characters sent to it by the parent process and print out the number of characters it received from the parent. The child process should not use the arguments to main() in any way whatsoever. The child should return normally and not have the parent kill the child.

Am i counting the arguments right? am i sending the arguments in one at a time, and am i reaping the child?

#include <sys/wait.h>
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <string.h>
#define size = 100;

int main(int argc, char *argv[])
{

    int i, count =0;
    int c;

    int     fdest[2];          // for pipe
    pid_t   pid;              //process IDs
    char    buffer[BUFSIZ];



    if (pipe(fdest) < 0)          /* attempt to create pipe */
        perror( "pipe" );

    if ((pid = fork()) < 0)  /* attempt to create child / parent process */

    {
        perror( "fork" );
    } 


    /* parent process */
    else if (pid > 0) { 

        close(fdest[0]);
        for (i=1; i < argc; ++i) 
        {
            for (c=0; c < strlen(argv[i]); ++c) {
                write(fdest[1], &argv[i][c], 1);
            }
        }

        close(fdest[1]);         
        wait(NULL);             
        exit(0);

    } else {   

        /* child Process */
        close(fdest[1]);

        while (read(fdest[0], &buffer, 1) > 0)
        {
            count++;
        }


        printf("\nchild: counted %d characters\n", count);

    }
    wait(NULL);
    exit(0);

}
+1  A: 

The second wait() is superfluous; the child has no children of its own to wait for. The second 'exit(0);' could be replaced by 'return(0);'. You could omit the previous 'exit(0);' too.

The '#define size = 100;' is unused, which is just as well since the '=' makes it unusable for most purposes (and the semi-colon is a bad idea too - seldom does a macro end with a semi-colon). It should be '#define size 100' or 'enum { size = 100 };'. Often, people use upper case names for 'manifest constants', hence 'enum { SIZE = 100 };.

If you are reading one character at a time, you really don't need a buffer of size BUFSIZ (which is usually 512 or larger).

Also, it is a bad idea to do 'for (c = 0; c < strlen(argv[c]); c++)' because that calculates the length of the string on each iteration. Replace it with either of these:

for (const char *str = argv[i]; *str != '\0'; str++)
    write(fdest, str, 1);

for (c = 0, len = strlen(argv[i]); c < len; c++)
      write(fdest[1], &argv[i][c], 1);

You close the unused ends of the pipes - that is a crucial step to making things work correctly.

The code seems to be counting correctly. It works off the shelf when I test it. Why are you suspicious that it does not work?

Jonathan Leffler