On Thu, Mar 2, 2023 at 3:02 PM Costas Argyris <costas.argy...@gmail.com> wrote: > > Thanks for the review. > > What is the next step please?
Somebody pushed the patch already. Richard. > Thanks, > Costas > > On Thu, 2 Mar 2023 at 10:08, Richard Biener <richard.guent...@gmail.com> > wrote: >> >> On Thu, Mar 2, 2023 at 10:21 AM Costas Argyris <costas.argy...@gmail.com> >> wrote: >> > >> > I forgot to mention that: >> > >> > 1) The CreateProcess documentation >> > >> > https://learn.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-createprocessa >> > >> > doesn't mention anything about taking ownership of this or any other >> > buffer passed to it. >> >> Thanks - thus the patch is OK. >> >> Thanks, >> Richard. >> >> > 2) The cmdline buffer gets created by the argv_to_cmdline function >> > >> > https://github.com/gcc-mirror/gcc/blob/master/libiberty/pex-win32.c#L339 >> > >> > which has this comment right above it: >> > >> > /* Return a Windows command-line from ARGV. It is the caller's >> > responsibility to free the string returned. */ >> > >> > Thanks, >> > Costas >> > >> > On Thu, 2 Mar 2023 at 07:32, Richard Biener <richard.guent...@gmail.com> >> > wrote: >> >> >> >> On Wed, Mar 1, 2023 at 7:14 PM Costas Argyris via Gcc-patches >> >> <gcc-patches@gcc.gnu.org> wrote: >> >> > >> >> > Hi >> >> > >> >> > It seems that the win32_spawn function in libiberty/pex-win32.c is >> >> > leaking >> >> > the cmdline buffer in 2/3 exit scenarios (it is only free'd in 1/3). >> >> > The >> >> > problem here is that the cleanup code is written 3 times, one at each >> >> > exit >> >> > scenario. >> >> > >> >> > The proposed attached refactoring has the cleanup code appearing just >> >> > once >> >> > and is executed for all exit scenarios, reducing the likelihood of such >> >> > leaks in the future. >> >> >> >> One could imagine that CreateProcess in case of success takes ownership of >> >> the buffer pointed to by cmdline? If you can confirm it is not then the >> >> patch >> >> looks OK to me. >> >> >> >> Thanks, >> >> Richard. >> >> >> >> > Thanks, >> >> > Costas
