Bug: fhandler.cc rev=1.116 source code level bug in fhandler_base::dup (fhandler_base *child)

Bug: fhandler.cc rev=1.116 source code level bug in fhandler_base::dup (fhandler_base *child)

[Leo Kuznetsov]

Hi,

I might be wrong but it looks like:

In the function below
------------------------
  if (get_nohandle ())
    nh = NULL;        // NULL == 0 and is a valid (but possibly closed) handle

------------------------
MUST BE
------------------------
  if (get_nohandle ())
    nh = (HANDLE)-1;  // or better yet INVALID_HANDLE_VALUE

=========================
Otherwise dup(dup(-1)) == dup(0) and
sets an error if handle 0 is closed.
(This is excatly the case with recursive invocation
of gmake jobs with stdio redirected to pipes)...

---------------------------------

FILE: fhandler.cc
LINE: ~886

int
fhandler_base::dup (fhandler_base *child)
{
  debug_printf ("in fhandler_base dup");

  HANDLE nh;
  if (get_nohandle ())
    nh = NULL;
  else if (!DuplicateHandle (hMainProc, get_handle(), hMainProc, &nh, 0, TRUE,
DUPLICATE_SAME_ACCESS))
    {
      system_printf ("dup(%s) failed, handle %x, %E",
     get_name (), get_handle());
      __seterrno ();
      return -1;
    }

  child->set_io_handle (nh);
  return 0;
}



--
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple
Bug reporting:         http://cygwin.com/bugs.html
Documentation:         http://cygwin.com/docs.html
FAQ:                   http://cygwin.com/faq/

Re: fhandler.cc rev=1.116 source code level bug in fhandler_base::dup (fhandler_base *child)

[Chris January]

> I might be wrong but it looks like:
>
> In the function below
> ------------------------
>   if (get_nohandle ())
>     nh = NULL;        // NULL == 0 and is a valid (but possibly closed)
handle
>
> ------------------------
> MUST BE
> ------------------------
>   if (get_nohandle ())
>     nh = (HANDLE)-1;  // or better yet INVALID_HANDLE_VALUE
>
I just wish I could figure out what causes the 'dup(/dev) failed' errors I
see about 30% of the time I click cygwin.bat.

Chris



--
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple
Bug reporting:         http://cygwin.com/bugs.html
Documentation:         http://cygwin.com/docs.html
FAQ:                   http://cygwin.com/faq/

Re: Bug: fhandler.cc rev=1.116 source code level bug in fhandler_base::dup (fhandler_base *child)

[Christopher Faylor]

On Wed, May 22, 2002 at 08:48:33AM -0700, Leo Kuznetsov wrote:
>Hi,
>
>I might be wrong but it looks like:
>
>In the function below
>------------------------
>  if (get_nohandle ())
>    nh = NULL;        // NULL == 0 and is a valid (but possibly closed) handle
>
>------------------------
>MUST BE
>------------------------
>  if (get_nohandle ())
>    nh = (HANDLE)-1;  // or better yet INVALID_HANDLE_VALUE
>
>=========================
>Otherwise dup(dup(-1)) == dup(0) and
>sets an error if handle 0 is closed.
>(This is excatly the case with recursive invocation
>of gmake jobs with stdio redirected to pipes)...

1) What's "gmake"?

2) Why shouldn't it "set an error" if handle 0 is closed?

3) Why would anyone do a dup(dup(-1))?

4) Why would setting nh to anything affect this code?  This code path is
   only invoked for /cygdrive or /proc style paths.

If you have a specific problem, you should provide a test case.  I don't
see any problems with recursive invocation of gmake jobs with stdio.

cgf

>---------------------------------
>
>FILE: fhandler.cc
>LINE: ~886
>
>int
>fhandler_base::dup (fhandler_base *child)
>{
>  debug_printf ("in fhandler_base dup");
>
>  HANDLE nh;
>  if (get_nohandle ())
>    nh = NULL;
>  else if (!DuplicateHandle (hMainProc, get_handle(), hMainProc, &nh, 0, TRUE,
>DUPLICATE_SAME_ACCESS))
>    {
>      system_printf ("dup(%s) failed, handle %x, %E",
>     get_name (), get_handle());
>      __seterrno ();
>      return -1;
>    }
>
>  child->set_io_handle (nh);
>  return 0;
>}
>
>
>
>--
>Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple
>Bug reporting:         http://cygwin.com/bugs.html
>Documentation:         http://cygwin.com/docs.html
>FAQ:                   http://cygwin.com/faq/

--
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple
Bug reporting:         http://cygwin.com/bugs.html
Documentation:         http://cygwin.com/docs.html
FAQ:                   http://cygwin.com/faq/

Re: Bug: fhandler.cc rev=1.116 source code level bug in fhandler_base::dup (fhandler_base *child)

[mike stump]

> 1) What's "gmake"?

GNU make.

> 3) Why would anyone do a dup(dup(-1))?

Because they can.  dup(-1) is defined to return -1, and otherwise not
do anything and set errno to EBADF.

> 4) Why would setting nh to anything affect this code?  This code path is
>    only invoked for /cygdrive or /proc style paths.
> 
> If you have a specific problem, you should provide a test case.  I don't
> see any problems with recursive invocation of gmake jobs with stdio.

compare:

	( exec <&-; make foo; )

with

	( exec <&-; make foo </dev/null; )

Makefile:

foo:
	echo me
	ls && echo me

This needs to work.  When it works, one will see an ls, when it fails,
one will get a core file.  On a real unix system (solaris for
example), it works just fine

--
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple
Bug reporting:         http://cygwin.com/bugs.html
Documentation:         http://cygwin.com/docs.html
FAQ:                   http://cygwin.com/faq/

Re: Bug: fhandler.cc rev=1.116 source code level bug in fhandler_base::dup (fhandler_base *child)

[Christopher Faylor]

On Wed, May 22, 2002 at 02:27:16PM -0700, mike stump wrote:
>> 1) What's "gmake"?
>
>GNU make.

Yeah, I sort of figured that.  Is this the make that comes with the
distribution or something you've built yourself?

>> 2) Why shouldn't it "set an error" if handle 0 is closed?

No answer here...

>> 3) Why would anyone do a dup(dup(-1))?
>
>Because they can.  dup(-1) is defined to return -1, and otherwise not
>do anything and set errno to EBADF.

I'm aware that you should be able to use a -1 but I can't imagine
writing a program which does anything useful with that.

However, it looks like there is a logic error in dup() where if it is
passed a -1, it will not reliably set an EBADF errno.  The fix does not
look like what you proposed, hoever.

>> 4) Why would setting nh to anything affect this code?  This code path is
>>    only invoked for /cygdrive or /proc style paths.

No answer here?

>> If you have a specific problem, you should provide a test case.  I don't
>> see any problems with recursive invocation of gmake jobs with stdio.
>
>compare:
>
>	( exec <&-; make foo; )
>
>with
>
>	( exec <&-; make foo </dev/null; )
>
>Makefile:
>
>foo:
>	echo me
>	ls && echo me
>
>This needs to work.  When it works, one will see an ls, when it fails,
>one will get a core file.  On a real unix system (solaris for
>example), it works just fine

When does one see a core file?  Is this an intermittent failure?  I don't
see a core file from this behavior.

cgf

--
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple
Bug reporting:         http://cygwin.com/bugs.html
Documentation:         http://cygwin.com/docs.html
FAQ:                   http://cygwin.com/faq/