Re: cygpath hangings: A fix - bash patch enclosed -- bash maintainer please note!

[Ronald Landheer-Cieslak <blytkerchan@users.sourceforge.net>]

I've just checked the patch a wee bit closer - it looks OK to me. If you want
to push it upstream, please go ahead :)

I'm wrapping up release -16 now. I'll make it available for upload on -apps 
ASAP.

Thanks,

rlc

On Sat, Oct 18, 2003 at 01:58:36AM -0400, Christopher Faylor wrote:
> On Wed, Oct 15, 2003 at 04:30:12PM -0400, Christopher Faylor wrote:
> >I just managed to duplicate the problem on my system at work.
> >
> >Stay tuned.
> 
> I managed to duplicate it at home by booting into W2K, too.  That meant
> I didn't have to feel guilty about working on this at work.  :-)
> 
> This should fix the problem.  Bash wasn't closing the read end of a pipe
> in some situations.  I'm not sure why that would cause some programs to
> hang but the following patch fixes the problem.  I think it provides
> more robust code than what was in bash previously, too.
> 
> Ronald, if you agree with this patch, could you release a new version of
> bash, ASAP?  If you don't agree with the patch, then please let me
> (aka the cygwin list) know soon since I'm going to be submitting it
> upstream ASAP.
> 
> cgf
> 
> 2003-10-18  Christopher Faylor  <cgf@redhat.com>
> 
> 	* subst.c (command_substitute): Guard against opening a pipe handle in
> 	stdin/stdout/stderr since they may be closed and keeping the pipe
> 	handle open in a subprocess will cause hangs.
> 
> --- subst.c.orig	2003-10-15 15:09:01.000000000 -0400
> +++ subst.c	2003-10-18 01:47:49.737056307 -0400
> @@ -3716,6 +3716,7 @@ command_substitute (string, quoted)
>    pid_t pid, old_pid, old_pipeline_pgrp;
>    char *istring;
>    int result, fildes[2], function_value;
> +  int i, closeit[3];
>  
>    istring = (char *)NULL;
>  
> @@ -3742,6 +3743,16 @@ command_substitute (string, quoted)
>    if (subst_assign_varlist == 0 || garglist == 0)
>      maybe_make_export_env ();	/* XXX */
>  
> +
> +  for (i = 0; i <= 2; i++)
> +    if (fcntl (i, F_GETFD, &result) != -1)
> +      closeit[i] = 0;
> +    else
> +      {
> +	open ("/dev/null", O_RDONLY);
> +	closeit[i] = 1;
> +      }
> +
>    /* Pipe the output of executing STRING into the current shell. */
>    if (pipe (fildes) < 0)
>      {
> @@ -3749,6 +3760,10 @@ command_substitute (string, quoted)
>        goto error_exit;
>      }
>  
> +  for (i = 0; i <= 2; i++)
> +    if (closeit[i])
> +      close (i);
> +
>    old_pid = last_made_pid;
>  #if defined (JOB_CONTROL)
>    old_pipeline_pgrp = pipeline_pgrp;
> @@ -3793,21 +3808,8 @@ command_substitute (string, quoted)
>  	  exit (EXECUTION_FAILURE);
>  	}
>  
> -      /* If standard output is closed in the parent shell
> -	 (such as after `exec >&-'), file descriptor 1 will be
> -	 the lowest available file descriptor, and end up in
> -	 fildes[0].  This can happen for stdin and stderr as well,
> -	 but stdout is more important -- it will cause no output
> -	 to be generated from this command. */
> -      if ((fildes[1] != fileno (stdin)) &&
> -	  (fildes[1] != fileno (stdout)) &&
> -	  (fildes[1] != fileno (stderr)))
> -	close (fildes[1]);
> -
> -      if ((fildes[0] != fileno (stdin)) &&
> -	  (fildes[0] != fileno (stdout)) &&
> -	  (fildes[0] != fileno (stderr)))
> -	close (fildes[0]);
> +      close (fildes[1]);
> +      close (fildes[0]);
>  
>        /* The currently executing shell is not interactive. */
>        interactive = 0;
> 
> --
> Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple
> Problem reports:       http://cygwin.com/problems.html
> Documentation:         http://cygwin.com/docs.html
> FAQ:                   http://cygwin.com/faq/

-- 
We all dream of being the darling of everybody's darling.

--
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple
Problem reports:       http://cygwin.com/problems.html
Documentation:         http://cygwin.com/docs.html
FAQ:                   http://cygwin.com/faq/