Restructure argument setup in hashbang

We do a few things here:
- Move search_path to the scope where it is used to make dependencies
  more clear.
- Have a separate variable to store the file name we eventually need to
  free and move the free logic to happen in a single place.

Both of this allows us to still free the name even if a fault is generated and
also avoids a compiler warning as we try to assign a 'const char*'
file_name_exec to a 'char *', making it more clear to what exactly we
need to free. I also believe 'error' in line 245 was not initialized in
case 'file_name_exec' is used and this fixes that too.
Message-ID: <20240121210757.1900938-1-flaviocruz@gmail.com>

1 files changed, 43 insertions, 48 deletions

diff --git a/exec/hashexec.c b/exec/hashexec.c
index a1072917..09536798 100644
--- a/exec/hashexec.c
+++ b/exec/hashexec.c
@@ -221,14 +221,14 @@ check_hashbang (struct execdata *e,
 
   if (! e->error)
     {
-      int free_file_name = 0; /* True if we should free FILE_NAME.  */
+      char * volatile file_name_to_free = NULL; /* Will free this if set.  */
       jmp_buf args_faulted;
       void fault_handler (int signo)
 	{ longjmp (args_faulted, 1); }
       error_t setup_args (struct hurd_signal_preemptor *preemptor)
 	{
 	  size_t namelen;
-	  char * volatile file_name = NULL;
+	  const char * volatile file_name = NULL;
 
 	  if (setjmp (args_faulted))
 	    file_name = NULL;
@@ -242,44 +242,42 @@ check_hashbang (struct execdata *e,
 		 named by ARGV[0] in the `PATH' environment variable
 		 might find it.  */
 
-	      error_t error;
-	      char *name;
-	      int free_name = 0; /* True if we should free NAME. */
-	      file_t name_file;
-	      mach_port_t fileid, filefsid;
-	      ino_t fileno;
-
-	      /* Search $PATH for NAME, opening a port NAME_FILE on it.
-		 This is encapsulated in a function so we can catch faults
-		 reading the user's environment.  */
-	      error_t search_path (struct hurd_signal_preemptor *preemptor)
+	      if (file_name_exec && file_name_exec[0] != '\0')
+		file_name = file_name_exec;
+	      else
 		{
-		  error_t err;
-		  char *path = envz_get (envp, envplen, "PATH"), *pfxed_name;
-
-		  if (! path)
+		  error_t error;
+		  file_t name_file;
+		  mach_port_t fileid, filefsid;
+		  ino_t fileno;
+		  char *name;
+		  /* Search $PATH for NAME, opening a port NAME_FILE on it.
+		     This is encapsulated in a function so we can catch faults
+		     reading the user's environment.  */
+		  error_t search_path (struct hurd_signal_preemptor *preemptor)
 		    {
-		      const size_t len = confstr (_CS_PATH, NULL, 0);
-		      path = alloca (len);
-		      confstr (_CS_PATH, path, len);
-		    }
+		      error_t err;
+		      char *path = envz_get (envp, envplen, "PATH"), *pfxed_name;
 
-		  err = hurd_file_name_path_lookup (user_port, user_fd, 0,
-						    name, path, O_EXEC, 0,
-						    &name_file, &pfxed_name);
-		  if (!err && pfxed_name)
-		    {
-		      name = pfxed_name;
-		      free_name = 1;
-		    }
+		      if (! path)
+			{
+			  const size_t len = confstr (_CS_PATH, NULL, 0);
+			  path = alloca (len);
+			  confstr (_CS_PATH, path, len);
+			}
 
-		  return err;
-		}
+		      err = hurd_file_name_path_lookup (user_port, user_fd, 0,
+							name, path, O_EXEC, 0,
+							&name_file, &pfxed_name);
+		      if (!err && pfxed_name)
+			{
+			  name = pfxed_name;
+			  file_name_to_free = pfxed_name;
+			}
+
+		      return err;
+		    }
 
-	      if (file_name_exec && file_name_exec[0] != '\0')
-		name = file_name_exec;
-	      else
-		{
 		  /* Try to locate the file.  */
 		  error = io_identity (file, &fileid, &filefsid, &fileno);
 		  if (error)
@@ -320,15 +318,10 @@ check_hashbang (struct execdata *e,
 		    }
 
 		  mach_port_deallocate (mach_task_self (), fileid);
-		}
 
-	      if (!error)
-		{
-		  file_name = name;
-		  free_file_name = free_name;
+		  if (!error)
+		    file_name = name;
 		}
-	      else if (free_name)
-		free (name);
 	    }
 
 	  if (file_name == NULL)
@@ -354,8 +347,9 @@ check_hashbang (struct execdata *e,
 	      mach_port_mod_refs (mach_task_self (), file,
 				  MACH_PORT_RIGHT_SEND, +1);
 
-	      file_name = alloca (100);
-	      sprintf (file_name, "/dev/fd/%d", fd);
+	      char *fd_file_name = alloca (100);
+	      sprintf (fd_file_name, "/dev/fd/%d", fd);
+	      file_name = fd_file_name;
 	    }
 
 	  /* Prepare the arguments to pass to the interpreter from the original
@@ -374,7 +368,7 @@ check_hashbang (struct execdata *e,
 	  if (new_argv == (caddr_t) -1)
 	    {
 	      e->error = errno;
-	      return e->error;
+	      goto end_setup_args;
 	    }
 	  else
 	    e->error = 0;
@@ -414,10 +408,11 @@ check_hashbang (struct execdata *e,
 	      memcpy (memcpy (n, arg, arg_len) + arg_len, file_name, namelen);
 	    }
 
-	  if (free_file_name)
-	    free (file_name);
+end_setup_args:
+	  if (file_name_to_free)
+	    free (file_name_to_free);
 
-	  return 0;
+	  return e->error;
 	}
 
       /* Set up the arguments.  */