bug-hurd
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[PATCH hurd 1/6] Restructure argument setup in hashbang


From: Flavio Cruz
Subject: [PATCH hurd 1/6] Restructure argument setup in hashbang
Date: Sun, 21 Jan 2024 16:07:52 -0500

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.
---
 exec/hashexec.c | 91 +++++++++++++++++++++++--------------------------
 1 file changed, 43 insertions(+), 48 deletions(-)

diff --git a/exec/hashexec.c b/exec/hashexec.c
index a107291..0953679 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.  */
-- 
2.39.2




reply via email to

[Prev in Thread] Current Thread [Next in Thread]