[C] Pipe that hangs

This is a discussion on [C] Pipe that hangs within the C Programming forums, part of the General Programming Boards category; Hi! I'm writing a little shell that behaves like old Thompson shell. I've a big problem with the creation of ...

  1. #1
    Registered User
    Join Date
    Jan 2007
    Posts
    7

    [C] Pipe that hangs

    Hi!

    I'm writing a little shell that behaves like old Thompson shell. I've a big problem with the creation of pipe mechanism. I must create pipe mechanism like UNIX shell (cmd1 | cmd2 | ... | cmdn). Below there is a portion of my function:

    Code:
    int j=0;
    for(i=0; i<res; ++i){
          if((pid = fork()) < 0){
             perror("fork error");
             exit(FORK_ERR);
          }
          if(pid == 0){  
             if(args!=NULL){
                free(args);
                args=NULL;
             } 
             args=(char **)calloc(MAX_ARGS,sizeof(char *));
             if(do_I_launch_parse(commands[i]) == 1)
                parse(commands[i],args);
             else{
                if(args[j]==NULL)
                   args[j]=(char *)calloc(strlen(commands[i])+1,sizeof(char));
                args[j]=strtok(commands[i]," ");
                ++j;
                while((s=strtok(NULL," ")) != NULL){
                   if(args[j]==NULL)
                      args[j]=(char *)calloc(strlen(commands[i])+1,sizeof(char));  
                   strcpy(args[j],s);
                   ++j;
                }
             }
             if(i==0){
                close(p1[0]);
                close(p2[1]);       
                close(p2[0]);
                if(dup2(p1[1],STDOUT_FILENO) == -1){
                   perror("dup error");
                   exit(DUP_ERR);
                }
                //close(p2[1]);
             }
             else if(i==res-1){
                if(i%2==0){
                   close(p2[1]);
                   close(p1[0]);
                   close(p1[1]);
                   if(dup2(p2[0],STDIN_FILENO) == -1){
                      perror("dup error");
                      exit(DUP_ERR);
                   }
                  // close(p2[0]);
                }
                else{
                   close(p1[1]);
                   close(p2[0]);
                   close(p2[1]);
                   if(dup2(p1[0],STDIN_FILENO) == -1){
                      perror("dup error");
                      exit(DUP_ERR);
                   }
                   //close(p1[0]);
                }
             } 
             else if(i%2==0){
                close(p2[1]);
                close(p1[0]);
                if(dup2(p2[0],STDIN_FILENO) == -1){
                   perror("dup error");
                   exit(DUP_ERR);
                }       
                //close(p2[0]);
                if(dup2(p1[1],STDOUT_FILENO) == -1){
                   perror("dup error");
                   exit(DUP_ERR);
                }       
                //close(p1[1]);
             }
             else{
                close(p2[0]);
                close(p1[1]);
                if(dup2(p1[0],STDIN_FILENO) == -1){
                   perror("dup error");
                   exit(DUP_ERR);
                }       
                //close(p1[0]);
                if(dup2(p2[1],STDOUT_FILENO) == -1){
                   perror("dup error");
                   exit(DUP_ERR);
                } 
                //close(p2[1]); 
             }
             ((args[0]==NULL) ? execlp(commands[i],commands[i],NULL) : execvp(commands[i],args));
             perror("exec failed");
             exit(EXEC_ERR); 
          }
          if(pid > 0){
             if (waitpid(pid,&status,0) == -1){
                if(errno!=EINTR){
                   perror("waitpid error");
                   exit(WAIT_ERR);
                }
                else{
                   kill(pid,SIGTERM);
                   do{
                      if (waitpid(pid,&status,0) != -1)
                         break;
                   }while(errno==EINTR);
                }
             }
          }
       }
    Ignore all the strings management. The problem (I hope) is in the pipe. If i launch a command like "ls -l | grep *.h", I receive this output:

    Code:
    /home/manugal/Progetto  $ ls -l | grep *.h
    -rw-r--r-- 1 manugal manugal  3364 2007-01-11 13:28 functions.h
    -rw-r--r-- 1 manugal manugal  3371 2007-01-11 13:10 functions.h~
    and after the output of second command, it hangs and it returns at prompt only if I press CTRL-C (it seems like that wait any data on pipe), but I don't understand why.

    In fact, in the code, if I add a line that set p[0] (that reads) in O_NONBLOCK mode, the output is:

    Code:
    /home/manugal/Progetto  $ ls -l | grep *.h
    -rw-r--r-- 1 manugal manugal  3364 2007-01-11 13:28 functions.h
    -rw-r--r-- 1 manugal manugal  3371 2007-01-11 13:10 functions.h~
    grep: (standard input): Resource temporarily not available
    Why? I've tried also to create two pipes to resolve the problem but nothing. Please, help me

    Thanks
    Last edited by Manugal; 01-12-2007 at 09:14 AM.

  2. #2
    and the hat of wrongness Salem's Avatar
    Join Date
    Aug 2001
    Location
    The edge of the known universe
    Posts
    32,751
    Erm, there's plenty of basics to clear up first

    > if(args!=NULL)
    > free(args);
    Except following the fork(), there are now two copies of the allocated memory, and you're only releasing one of them. It's a memory leak in the parent.

    > args=(char **)calloc(MAX_ARGS,sizeof(char *));
    1. Don't cast the return result of malloc / calloc etc (see the FAQ
    2. All-bits zero implied by calloc doesn't necessarily mean it is a valid and NULL pointer - see http://c-faq.com/malloc/calloc.html

    > args[j]=(char *)calloc(strlen(commands[i])+1,sizeof(char));
    > args[j]=strtok(commands[i]," ");
    Another memory leak. strtok() returns a pointer to the innards of your commands string, and overwrites part of that string. You certainly can't then do free( args[j] ) later on.

    Some of your strlen() calls are highly dubious as well, given that strtok() is modifying the same string as well.

    > close(p2[1]);
    > close(p2[0]);
    Closing both ends of the same pipe would seem to make it useless as a pipe.
    Also, use some constants to identify the READ end and the WRITE end of the pipe.
    Also, where are all these pipes being created?

    I don't understand why you have 5 separate cases for closing / dup'ing various ends of various pipes. I can see why you might need 3 (for start, middle and end) of the whole pipeline.
    If you dance barefoot on the broken glass of undefined behaviour, you've got to expect the occasional cut.
    If at first you don't succeed, try writing your phone number on the exam paper.
    I support http://www.ukip.org/ as the first necessary step to a free Europe.

  3. #3
    Registered User
    Join Date
    Jan 2007
    Posts
    7
    Thanks for the reply

    Ok. It's obvious that you say these things to me. I post only the code part that interesting this functionality. Naturally It's not the whole program. This is only a for cycle of a function that it would have to execute pipe. Let's start:

    It's a memory leak in the parent.
    No, because when the function terminate, it return to main function that free all dinamically allocated variables. That control is there, because args sometimes has dirty values.

    args=(char **)calloc(MAX_ARGS,sizeof(char *));
    Perhaps here you have reason , but I am accustomed to cast when i call calloc or malloc.

    Another memory leak. strtok() returns a pointer to the innards of your commands string, and overwrites part of that string. You certainly can't then do free( args[j] ) later on.
    When I free memory on a pointer to pointer I call free() function in this manner:

    Code:
    if(args!=NULL){
       free(args);
       args=NULL;
    }
    but I don't call free for every string at which point args. It's wrong?

    Some of your strlen() calls are highly dubious as well, given that strtok() is modifying the same string as well.
    Because I've not post the whole program. I use all these strings management to subdivide arguments. For example if I have "ls -l *.c" I divide this string in "ls" "-l" "*.c". Then there are other problem, for example "*.c" what does mean? Therefore I must to call parse function that use wordexp to expand "*.c" etc. etc. . It's quite complex. However, to the end, the string passed to exec function is correct (in fact the command is executed correctly).

    Closing both ends of the same pipe would seem to make it useless as a pipe.
    But I have only closed the ends of pipe that I not using in that part of code.

    Finally, I've 5 separate cases because is the only method that it has come in my mind


    I hope that it's all clear.
    Last edited by Manugal; 01-12-2007 at 12:31 PM.

  4. #4
    CSharpener vart's Avatar
    Join Date
    Oct 2006
    Location
    Rishon LeZion, Israel
    Posts
    6,484
    Quote Originally Posted by Manugal
    When I free memory on a pointer to pointer I call free() function in this manner:

    Code:
    if(args!=NULL){
       free(args);
       args=NULL;
    }
    but I don't call free for every string at which point args. It's wrong?
    yes, it is wrong.
    The basic rule is if you got some pointer from calloc/malloc - you should call free for this pointer (I'm not considering now the realloc case...)

    so if you called malloc to initialize args[i] - you save this pointer till the memory is of no use anymore and call free for it.
    if you modifying the pointer and loosing the original value - you got memory leak.
    if you store the pointer in the dynamic array of pointers and then free the memory where array is stored without freeing each pointer beforehead - it is a memory leak too.
    The first 90% of a project takes 90% of the time,
    the last 10% takes the other 90% of the time.

  5. #5
    Registered User
    Join Date
    Jan 2007
    Posts
    7
    Ok. In fact now I've corrected this wrong memory management. But the problem remains.

  6. #6
    ATH0 quzah's Avatar
    Join Date
    Oct 2001
    Posts
    14,826
    How about you stop closing all of those pipes, and try using them for something like reading / writing, and display what you get from them? You know, to debug the issue.

    Quzah.
    Hope is the first step on the road to disappointment.

  7. #7
    Registered User
    Join Date
    Jan 2007
    Posts
    7
    I've just tried it. I've created a buffer buf[4096] and I tried to read from parent from p1[0] and p2[0]. It doesn't read anything. They are empty. In fact I don't understand how the two program that I tried in pipe (ls -l | grep *.h) succeed to communicate. :|

  8. #8
    ATH0 quzah's Avatar
    Join Date
    Oct 2001
    Posts
    14,826
    You could always make a simple version that only has a pair of pipes. Make sure you understand how they communicate. Prepend a bit of info from each one to the buffer, send it back and forth as each reads the message. Start small. Get that working. Then try adding one more child to the mix.


    Quzah.
    Hope is the first step on the road to disappointment.

  9. #9
    Registered User
    Join Date
    Jan 2007
    Posts
    7
    Ok. I'll try it and I'll return here for news. Thanks a lot.

  10. #10
    Registered User
    Join Date
    Jan 2007
    Posts
    7
    Ok. I've tried it. With two programs no problem (I play start the first program in child and the second in parent). With one more command, return the old problem. Now it's the code (res variable is the count of programs in the pipe). It's similarly to that one before, but now I also have the error message that give me gdb:

    Code:
    int j=0;
    ....
    
    if((pid=fork()) < 0){
       perror("fork error");
       exit(FORK_ERR);
    }
    for(i=0; i<res; ++i){
       if(pid == 0){
          if(do_I_launch_parse(commands[i]) == 1)
                parse(commands[i],args);
             else{
                if(args[j]==NULL)
                   args[j]=(char *)calloc(strlen(commands[i])+1,sizeof(char));
                args[j]=strtok(commands[i]," ");
                ++j;
                while((s=strtok(NULL," ")) != NULL){
                   if(args[j]==NULL)
                      args[j]=(char *)calloc(strlen(commands[i])+1,sizeof(char));  
                   strcpy(args[j],s);
                   ++j;
                }
             }
             if(i==0){
                close(p[0]);
                dup2(p[1],STDOUT_FILENO);
                close(p[1]);
             }
             else if(i==res-1){
                close(p[1]);
                dup2(p[0],STDIN_FILENO);
                close(p[0]);
             }
             else{
                dup2(p[0],STDIN_FILENO);
                dup2(p[1],STDOUT_FILENO);
             }
             if((pid1=fork())==0){ /* second child */
                ((args[0]==NULL) ? execlp(commands[i],commands[i],NULL) : execvp(commands[i],args));
                perror("exec failed");
                exit(EXEC_ERR);
             }
             else if(pid1 < 0){
                perror("fork error");
                exit(FORK_ERR);
             }
             if (waitpid(pid1,&status,0) == -1){
                if(errno!=EINTR){
                   perror("waitpid error");
                   exit(WAIT_ERR);
                }
                else{
                   kill(pid1,SIGTERM);
                   do{
                      if (waitpid(pid1,&status,0) != -1)
                         break;
                   }while(errno==EINTR);
                }
             }
          }
          }
          if(pid > 0){
             if (waitpid(pid,&status,0) == -1){
                if(errno!=EINTR){
                   perror("waitpid error");
                   exit(WAIT_ERR);
                }
                else{
                   kill(pid,SIGTERM);
                   do{
                      if (waitpid(pid,&status,0) != -1)
                         break;
                   }while(errno==EINTR);
                }
             }
           }
    With gdb when I arrive in the second child it (when I on the second command), it would execute the second command, but it no return any output and when it calls the wait in parent (the first child), it give to me this message two times:

    Program received signal SIGTTIN, Stopped (tty input).

    Certainly, it returned this message also with the previous code that I post (but in that case in parent directly). Why it gives me this message?

    P.S.: I had not using any buffer, because I don't know how and in which point capture data.
    Last edited by Manugal; 01-13-2007 at 07:31 AM.

  11. #11
    Registered User
    Join Date
    Jan 2007
    Posts
    7
    Please.

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. cont of IPC using PIPE
    By BMathis in forum C Programming
    Replies: 1
    Last Post: 03-15-2009, 06:16 PM
  2. Pipe class.
    By eXeCuTeR in forum Linux Programming
    Replies: 8
    Last Post: 08-21-2008, 04:44 AM
  3. Pipe(): Interprocess or Intraprocess comm?
    By @nthony in forum C Programming
    Replies: 2
    Last Post: 03-28-2007, 08:27 PM
  4. Clearing a pipe used in IPC
    By cbgb in forum C Programming
    Replies: 5
    Last Post: 09-26-2006, 04:59 PM
  5. Having trouble with a named pipe
    By crazeinc in forum C Programming
    Replies: 2
    Last Post: 05-13-2005, 02:00 AM

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21