Thanks for the review. What is the next step please?
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 >
