public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] libiberty: fix memory leak in pex-win32.c and refactor
@ 2023-03-01 18:07 Costas Argyris
  2023-03-02  7:32 ` Richard Biener
  0 siblings, 1 reply; 6+ messages in thread
From: Costas Argyris @ 2023-03-01 18:07 UTC (permalink / raw)
  To: gcc-patches


[-- Attachment #1.1: Type: text/plain, Size: 425 bytes --]

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.

Thanks,
Costas

[-- Attachment #2: 0001-libiberty-fix-memory-leak-in-pex-win32.c-and-refacto.patch --]
[-- Type: text/x-patch, Size: 2115 bytes --]

From ca1ff7b48db2e30963bd4c0e08ecd6653214a5db Mon Sep 17 00:00:00 2001
From: Costas Argyris <costas.argyris@gmail.com>
Date: Sun, 26 Feb 2023 16:34:11 +0000
Subject: [PATCH] libiberty: fix memory leak in pex-win32.c and refactor

Fix memory leak of cmdline buffer and refactor to have
cleanup code appear once for all exit cases.
---
 libiberty/pex-win32.c | 33 +++++++++++----------------------
 1 file changed, 11 insertions(+), 22 deletions(-)

diff --git a/libiberty/pex-win32.c b/libiberty/pex-win32.c
index 02d3a3e839b..82303947de4 100644
--- a/libiberty/pex-win32.c
+++ b/libiberty/pex-win32.c
@@ -577,14 +577,12 @@ win32_spawn (const char *executable,
 	     LPSTARTUPINFO si,
 	     LPPROCESS_INFORMATION pi)
 {
-  char *full_executable;
-  char *cmdline;
+  char *full_executable = NULL;
+  char *cmdline = NULL;
+  pid_t pid = (pid_t) -1;
   char **env_copy;
   char *env_block = NULL;
 
-  full_executable = NULL;
-  cmdline = NULL;
-
   if (env)
     {
       int env_size;
@@ -622,13 +620,13 @@ win32_spawn (const char *executable,
 
   full_executable = find_executable (executable, search);
   if (!full_executable)
-    goto error;
+    goto exit;
   cmdline = argv_to_cmdline (argv);
   if (!cmdline)
-    goto error;
+    goto exit;
     
   /* Create the child process.  */  
-  if (!CreateProcess (full_executable, cmdline, 
+  if (CreateProcess (full_executable, cmdline, 
 		      /*lpProcessAttributes=*/NULL,
 		      /*lpThreadAttributes=*/NULL,
 		      /*bInheritHandles=*/TRUE,
@@ -638,26 +636,17 @@ win32_spawn (const char *executable,
 		      si,
 		      pi))
     {
-      free (env_block);
-
-      free (full_executable);
-
-      return (pid_t) -1;
+      CloseHandle (pi->hThread);
+      pid = (pid_t) pi->hProcess;
     }
-
+  
+ exit:
   /* Clean up.  */
-  CloseHandle (pi->hThread);
-  free (full_executable);
-  free (env_block);
-
-  return (pid_t) pi->hProcess;
-
- error:
   free (env_block);
   free (cmdline);
   free (full_executable);
 
-  return (pid_t) -1;
+  return pid;
 }
 
 /* Spawn a script.  This simulates the Unix script execution mechanism.
-- 
2.30.2


^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-03-03 11:25 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-01 18:07 [PATCH] libiberty: fix memory leak in pex-win32.c and refactor Costas Argyris
2023-03-02  7:32 ` Richard Biener
2023-03-02  9:21   ` Costas Argyris
2023-03-02 10:08     ` Richard Biener
2023-03-02 14:02       ` Costas Argyris
2023-03-03 11:25         ` Richard Biener

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).