Re: [RFA] win32-nat.c: Restore wrongly removed code that closes two handles returned by CreateProcess

[Christopher Faylor <cgf-use-the-mailinglist-please@sourceware.org>]

On Fri, Jan 11, 2008 at 06:20:48PM +0100, Pierre Muller wrote:
> This patch restores two lines of code that I wrongfully
>removed in a previous patch about problems related to 
>closing of handles that the system should close.
>
>The applied patch is:
>http://sourceware.org/ml/gdb-patches/2007-11/msg00481.html
>
>  In that patch,
> I also removed two calls to CloseHandle functions for the two
>handles that are returned by the CreateProcess function.
>To make things worse, I forgot to mention that
>removal in the corresponding ChangeLog.
>
>The wrong part of the patch is in win32_create_inferior
>@@ -1886,9 +1893,6 @@ win32_create_inferior (char *exec_file,
>     error (_("Error creating process %s, (error %d)."),
>           exec_file, (unsigned) GetLastError ());
>
>-  CloseHandle (pi.hThread);
>-  CloseHandle (pi.hProcess);
>-
>   if (useshell && shell[0] != '\0')
>     saw_create = -1;
>   else
>
>  I was supposing that the handles returned by
>CreateProcess in the ProcessInformation structure
>were the same as the handles given in the 
>initial CREATE_PROCESS_DEBUG_EVENT.
>  But these two handles are completely separate, as
>printing out the values enabled me to see. The handles
>returned by CreateProcess are given to any program
>launching another executable, while the other handles are debugger
>specific.
>
>   The purpose of that patch was about avoiding to close the handles
>that are given on debug events, as stated in the windows API 
>documentation. But the same docs do specify that
>the two handles returned in the ProcessInformation structure
>need to be closed.
>
>  I ran the testsuite on that patch and found no
>change, but the missing CloseHandle calls might still
>have some influence on other cases.
>  Luckily no gdb release contains that error.
>
>  Christopher, should I modify the ChangeLog-2007 entry
>that contains the bad removal of that code, as
>the entry is incomplete?

No, AFAIK, you are not supposed to retroactively change a ChangeLog
entry like that.

>  Sorry about this mistake,

No need to apologize! I'm glad you caught this and I'm glad that you
researched your earlier changes.  I actually stumbled across your
initial bug separately last week while debugging something with an older
version of gdb.  So, I'm happy that you fixed it.

>ChangeLog entry:
>
>2008-01-11  Pierre Muller  <muller@ics.u-strasbg.fr> 
>
>	* win32-nat.c (win32_create_inferior): Restore
>	code calling CloseHandle on ProcessInformation structure.

Please check in.  I'm sorry that I didn't catch this when I reviewed
your change.  It's obvious in perfect 20-20 hindsight.

cgf