From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 17459 invoked by alias); 11 Jan 2008 18:32:16 -0000 Received: (qmail 17445 invoked by uid 22791); 11 Jan 2008 18:32:15 -0000 X-Spam-Check-By: sourceware.org Received: from pool-96-233-71-153.bstnma.fios.verizon.net (HELO ednor.cgf.cx) (96.233.71.153) by sourceware.org (qpsmtpd/0.31) with ESMTP; Fri, 11 Jan 2008 18:31:48 +0000 Received: by ednor.cgf.cx (Postfix, from userid 201) id C777F2B352; Fri, 11 Jan 2008 13:31:46 -0500 (EST) Date: Fri, 11 Jan 2008 18:32:00 -0000 From: Christopher Faylor To: gdb-patches@sourceware.org, Pierre Muller Subject: Re: [RFA] win32-nat.c: Restore wrongly removed code that closes two handles returned by CreateProcess Message-ID: <20080111183146.GA27708@ednor.casa.cgf.cx> Mail-Followup-To: gdb-patches@sourceware.org, Pierre Muller References: <001301c85476$4eff2fc0$ecfd8f40$@u-strasbg.fr> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <001301c85476$4eff2fc0$ecfd8f40$@u-strasbg.fr> User-Agent: Mutt/1.5.16 (2007-06-09) Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2008-01/txt/msg00281.txt.bz2 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 > > * 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