[system-tools-backends-clone] Bugs 519273 & 346342 - Clean Utils::File::run() logic by using array of arguments



commit ee59351bfa31ee60779a20a79b1b49f308a777b0
Author: Milan Bouchet-Valat <nalimilan club fr>
Date:   Thu Sep 17 22:35:01 2009 +0200

    Bugs 519273 & 346342 - Clean Utils::File::run() logic by using array of arguments
    
    Using system() with a string instead of an array raises various issues when strings passed include special chars. Overall, it's claner and safer to use an array everywhere. This requires us to use our own execution code since system() does not accept setting STDOUT, STDERR, LC_ALL, and starting processes in the background when using an array. Note that run_backticks() still use the old logic, which is safe(r) because those commandlines are static. However, this commit also cleans get_cmd_path() by merging do_get_cmd_path() into it, and removing duplicate settings like $PATH.

 Init/Services.pm  |   30 ++++++------
 Network/Hosts.pm  |    2 +-
 Network/Ifaces.pm |    3 +-
 Shares/NFS.pm     |    4 +-
 Shares/SMB.pm     |    2 +-
 Time/NTP.pm       |    2 +-
 Time/TimeDate.pm  |   18 ++++----
 Users/Groups.pm   |   69 ++++++++++++++----------------
 Users/Users.pm    |  124 +++++++++++++++++++++++++++-------------------------
 Utils/File.pm     |   86 ++++++++++++++++++++----------------
 Utils/Report.pm   |    2 +-
 11 files changed, 175 insertions(+), 167 deletions(-)
---
diff --git a/Init/Services.pm b/Init/Services.pm
index f847dbc..ce5dbac 100644
--- a/Init/Services.pm
+++ b/Init/Services.pm
@@ -232,7 +232,7 @@ sub run_sysv_initd_script
 
   if (-f "$initd_path/$service")
   {
-    if (&Utils::File::run ("$initd_path/$service $arg") == 0)
+    if (&Utils::File::run ("$initd_path/$service", $arg) == 0)
     {
       &Utils::Report::do_report ("service_sysv_op_success", $service, $arg);
       &Utils::Report::leave ();
@@ -589,15 +589,15 @@ sub run_bsd_script
   if (! -x $service)
   {
     $chmod = 1;
-    &Utils::File::run ("chmod ugo+x $service");
+    &Utils::File::run ("chmod", "ugo+x", $service);
   }
 
-  &Utils::File::run ("$service $arg", 1);
+  &Utils::File::run_bg ($service, $arg);
 
   # return it to it's normal state
   if ($chmod)
   {
-    &Utils::File::run ("chmod ugo-x $service");
+    &Utils::File::run ("chmod", "ugo-x", $service);
   }
 }
 
@@ -625,13 +625,13 @@ sub set_bsd_services
 
     if ($status == $SERVICE_START)
     {
-      &Utils::File::run ("chmod ugo+x $script");
+      &Utils::File::run ("chmod", "ugo+x", $script);
       &run_bsd_script ($script, "start");
     }
     else
     {
       &run_bsd_script ($script, "stop");
-      &Utils::File::run ("chmod ugo-x $script");
+      &Utils::File::run ("chmod", "ugo-x", $script);
     }
   }
 }
@@ -778,7 +778,7 @@ sub run_gentoo_script
 
   if (&gentoo_service_exists ($service))
   {
-    if (!&Utils::File::run ("/etc/init.d/$service $arg"))
+    if (!&Utils::File::run ("/etc/init.d/$service", $arg))
     {
       &Utils::Report::do_report ("service_sysv_op_success", $service, $arg);
       &Utils::Report::leave ();
@@ -804,13 +804,13 @@ sub set_gentoo_service_status
 
   if ($status == $SERVICE_START)
   {
-    &Utils::File::run ("rc-update add $script $rl");
+    &Utils::File::run ("rc-update", "add", $script, $rl);
     &run_gentoo_script ($script, "start");
   }
   else
   {
     &run_gentoo_script ($script, "stop");
-    &Utils::File::run ("rc-update del $script $rl");
+    &Utils::File::run ("rc-update", "del", $script, $rl);
   }
 }
 
@@ -909,7 +909,7 @@ sub run_rcng_script
 
   &Utils::Report::enter ();
 
-  if (!&Utils::File::run ("/etc/rc.d/$service $arg"))
+  if (!&Utils::File::run ("/etc/rc.d/$service", $arg))
   {
     &Utils::Report::do_report ("service_sysv_op_success", $service, $arg);
     &Utils::Report::leave ();
@@ -1092,7 +1092,7 @@ sub set_suse_services
     $rllist = "";
     %configured_runlevels = {};
 
-    &Utils::File::run ("insserv -r $script");
+    &Utils::File::run ("insserv", "-r", $script);
 
     foreach $rl (@$runlevels)
     {
@@ -1110,7 +1110,7 @@ sub set_suse_services
     {
       $rllist =~ s/,$//;
 
-      &Utils::File::run ("insserv $script,start=$rllist");
+      &Utils::File::run ("insserv", $script, "start=$rllist");
     }
 
     if (!$configured_runlevels{$default_runlevel})
@@ -1146,7 +1146,7 @@ sub run_smf_svcadm
 
   if (&smf_service_exists ($service))
   {
-    if (!&Utils::File::run ("svcadm $op{$arg} $service"))
+    if (!&Utils::File::run ("svcadm", $op{$arg}, $service))
     {
       &Utils::Report::do_report ("service_sysv_op_success", $service, $arg);
       &Utils::Report::leave ();
@@ -1245,11 +1245,11 @@ sub set_smf_service_status
 
   if ($active == $SERVICE_START)
   {
-    &Utils::File::run ("svcadm enable -s $service");
+    &Utils::File::run ("svcadm", "enable", "-s", $service);
   }
   else
   {
-    &Utils::File::run ("svcadm disable -s $service");
+    &Utils::File::run ("svcadm", "disable", "-s", $service);
   }
 }
 
diff --git a/Network/Hosts.pm b/Network/Hosts.pm
index 38b3ed1..94ffdb5 100644
--- a/Network/Hosts.pm
+++ b/Network/Hosts.pm
@@ -29,7 +29,7 @@ sub run_hostname
 
   &Utils::Report::enter ();
   &Utils::Report::do_report ("network_hostname_set", $hostname);
-  &Utils::File::run ("hostname $hostname");
+  &Utils::File::run ("hostname", $hostname);
   &Utils::Report::leave ();
 }
 
diff --git a/Network/Ifaces.pm b/Network/Ifaces.pm
index d847bd0..a105c78 100644
--- a/Network/Ifaces.pm
+++ b/Network/Ifaces.pm
@@ -126,8 +126,7 @@ sub get_linux_wireless_ifaces
   my ($fd, $line);
   my (@ifaces, $command);
 
-  $command = &Utils::File::get_cmd_path ("iwconfig");
-  open $fd, "$command |";
+  $fd = &Utils::File::run_pipe_read ("iwconfig");
   return @ifaces if $fd eq undef;
 
   while (<$fd>)
diff --git a/Shares/NFS.pm b/Shares/NFS.pm
index c7a42e8..723cbcc 100644
--- a/Shares/NFS.pm
+++ b/Shares/NFS.pm
@@ -374,8 +374,8 @@ sub set
 
   if ($Utils::Backend::tool{"system"} eq "SunOS")
   {
-    &Utils::File::run ("unshareall -F nfs");
-    &Utils::File::run ("shareall -F nfs");
+    &Utils::File::run ("unshareall", "-F", "nfs");
+    &Utils::File::run ("shareall", "-F", "nfs");
   }
 }
 
diff --git a/Shares/SMB.pm b/Shares/SMB.pm
index eb83c3c..ae740a7 100644
--- a/Shares/SMB.pm
+++ b/Shares/SMB.pm
@@ -317,7 +317,7 @@ sub set_smb_users
     {
       # User deleted
       $user = $old_hash{$i};
-      &Utils::File::run ("pdbedit --delete -u $$user[0]");
+      &Utils::File::run ("pdbedit", "--delete", "-u", $$user[0]);
     }
   }
 }
diff --git a/Time/NTP.pm b/Time/NTP.pm
index 364db26..39f7331 100644
--- a/Time/NTP.pm
+++ b/Time/NTP.pm
@@ -158,7 +158,7 @@ sub apply_ntp_date
 
   # run ntpdate, this will only be effective
   # when there isn't any NTP server running
-  &Utils::File::run ("ntpdate -b $servers") if ($servers);
+  &Utils::File::run ("ntpdate", "-b", $servers) if ($servers);
 }
 
 sub get
diff --git a/Time/TimeDate.pm b/Time/TimeDate.pm
index 19c5364..bf5ab45 100644
--- a/Time/TimeDate.pm
+++ b/Time/TimeDate.pm
@@ -43,7 +43,7 @@ sub get_utc_time
 sub change_timedate
 {
   my ($time) = @_;
-  my ($command);
+  my (@command);
 
   my $system_table = {
     "Linux"   => "date -u %02d%02d%02d%02d%04d.%02d",
@@ -51,13 +51,13 @@ sub change_timedate
     "SunOS"   => "date -u %02d%02d%02d%02d%04d.%02d",
   };
 
-  $command = sprintf ($$system_table {$Utils::Backend::tool{"system"}},
-                      $$time{"month"} + 1, $$time{"monthday"},
-                      $$time{"hour"},  $$time{"minute"}, 
-                      $$time{"year"},  $$time{"second"});
+  @command = ($$system_table {$Utils::Backend::tool{"system"}},
+              $$time{"month"} + 1, $$time{"monthday"},
+              $$time{"hour"}, $$time{"minute"},
+              $$time{"year"}, $$time{"second"});
 
-  &Utils::Report::do_report ("time_localtime_set", $command);
-  return &Utils::File::run ($command);
+  &Utils::Report::do_report ("time_localtime_set", @command);
+  return &Utils::File::run (@command);
 }
 
 sub set_utc_time
@@ -73,7 +73,7 @@ sub set_utc_time
 
 sub time_sync_hw_from_sys
 {
-  &Utils::File::run ("hwclock --systohc") if ($$tool{"system"} eq "Linux");
+  &Utils::File::run ("hwclock", "--systohc") if ($$tool{"system"} eq "Linux");
   return 0;
 }
 
@@ -116,7 +116,7 @@ sub get_timezone
     $size_test = (stat("$zoneinfo_dir/$zone"))[7];
     if ($size_test eq $size_search)
     {
-      if (!&Utils::File::run ("diff $zoneinfo_dir/$zone $local_time_file"))
+      if (!&Utils::File::run ("diff", "$zoneinfo_dir/$zone", $local_time_file))
       {
         # Found a match.
         last;
diff --git a/Users/Groups.pm b/Users/Groups.pm
index c19053f..facb7a6 100644
--- a/Users/Groups.pm
+++ b/Users/Groups.pm
@@ -52,15 +52,15 @@ sub del_group
 
   if ($Utils::Backend::tool{"system"} eq "FreeBSD")
   {
-    $command = "$cmd_pw groupdel -n \'" . $$group[$LOGIN] . "\'";
+    @command = ($cmd_pw, "groupdel", "-n", $$group[$LOGIN]);
   }
   else
   {
-    $command  = ($cmd_delgroup) ? $cmd_delgroup : $cmd_groupdel;
-    $command .= " \'" . $$group[$LOGIN] . "\'";
+    @command  = (($cmd_delgroup) ? $cmd_delgroup : $cmd_groupdel,
+                 $$group[$LOGIN]);
   }
 
-  &Utils::File::run ($command);
+  &Utils::File::run (@command);
 }
 
 # This is only for Linux and SunOS,
@@ -68,7 +68,7 @@ sub del_group
 sub add_user_to_group
 {
   my ($group, $user) = @_;
-  my ($command);
+  my (@command);
 
   if ($Utils::Backend::tool{"system"} eq "SunOS")
   {
@@ -84,14 +84,14 @@ sub add_user_to_group
     $groups =~ s/^,//;
     $groups =~ s/,$//;
 
-    $command = "$cmd_usermod -G $groups $user";
+    @command = ($cmd_usermod, "-G", $groups, $user);
   }
   else
   {
-    $command = "$cmd_gpasswd -a \'" . $user . "\' " . $group;
+    @command = ($cmd_gpasswd, "-a", $user, $group);
   }
 
-  &Utils::File::run ($command);
+  &Utils::File::run (@command);
 }
 
 # This is only for Linux and SunOS,
@@ -99,11 +99,11 @@ sub add_user_to_group
 sub delete_user_from_group
 {
   my ($group, $user) = @_;
-  my ($command);
+  my (@command);
 
   if ($Utils::Backend::tool{"system"} eq "SunOS")
   {
-    my ($groups, @arr);
+    my ($groups, @groups_arr);
 
     $groups = &Utils::File::run_backtick ("groups $user");
     $groups =~ s/.*://;
@@ -112,52 +112,47 @@ sub delete_user_from_group
     # delete the user
     $groups =~ s/[ \t]+$group//;
 
-    @arr = split (/ /, $groups);
-    $groups = join (',', @arr);
-    $groups =~ s/^,//;
-    $groups =~ s/,$//;
+    @groups_arr = split (/ /, $groups);
     
-    $command = "$cmd_usermod -G $groups $user";
+    @command = ($cmd_usermod, "-G", @groups_arr, $user);
   }
   else
   {
-    $command = "$cmd_gpasswd -d \'" . $user . "\' \'" . $group . "\'";
+    @command = ($cmd_gpasswd, "-d", $user, $group);
   }
 
-  &Utils::File::run ($command);
+  &Utils::File::run (@command);
 }
 
 sub add_group
 {
   my ($group) = @_;
-  my ($u, $user, $users);
+  my ($u, $user, @users);
 
   $u = $$group[$USERS];
 
   if ($Utils::Backend::tool{"system"} eq "FreeBSD")
   {
-    $users = join (",", sort @$u);
+    @users = sort @$u;
       
-    $command = "$cmd_pw groupadd -n \'" . $$group[$LOGIN] .
-      "\' -g \'" . $$group[$GID] .
-      "\' -M \'" . $users . "\'";
+    @command = ($cmd_pw, "groupadd", "-n", $$group[$LOGIN],
+                                     "-g", $$group[$GID],
+                                     "-M", @users);
 
-    &Utils::File::run ($command);
+    &Utils::File::run (@command);
   }
   else
   {
     if ($cmd_addgroup)
     {
-      $command = "$cmd_addgroup " .
-          "--gid \'" . $$group[$GID] . "\' " . $$group[$LOGIN];
+      @command = ($cmd_addgroup, "--gid", $$group[$GID], $$group[$LOGIN]);
     }
     else
     {
-      $command = "$cmd_groupadd -g \'" . $$group[$GID] .
-          "\' " . $$group[$LOGIN];
+      @command = ($cmd_groupadd, "-g", $$group[$GID], $$group[$LOGIN]);
     }
 
-    &Utils::File::run ($command);
+    &Utils::File::run (@command);
 
     foreach $user (sort @$u)
     {
@@ -178,20 +173,20 @@ sub change_group
     $users_arr = $$new_group[$USERS];
     $str = join (",", sort @$users_arr);
 
-    $command = "$cmd_pw groupmod -n \'" . $$old_group[$LOGIN] .
-        "\' -g \'" . $$new_group[$GID] .
-        "\' -l \'" . $$new_group[$LOGIN] .
-        "\' -M \'" . $str . "\'";
+    @command = ($cmd_pw, "groupmod", "-n", $$old_group[$LOGIN],
+                                     "-g", $$new_group[$GID],
+                                     "-l", $$new_group[$LOGIN],
+                                     "-M", $str);
 
-    &Utils::File::run ($command);
+    &Utils::File::run (@command);
   }
   else
   {
-    $command = "$cmd_groupmod -g \'" . $$new_group[$GID] .
-        "\' -n \'" . $$new_group[$LOGIN] . "\' " .
-        "\'" . $$old_group[$LOGIN] . "\'";
+    @command = ($cmd_groupmod, "-g", $$new_group[$GID],
+                               "-n", $$new_group[$LOGIN],
+                                     $$old_group[$LOGIN]);
   
-    &Utils::File::run ($command);
+    &Utils::File::run (@command);
 
     # Let's see if the users that compose the group have changed.
     if (!Utils::Util::struct_eq ($$new_group[$USERS], $$old_group[$USERS]))
diff --git a/Users/Users.pm b/Users/Users.pm
index bd0603a..bfc6d96 100644
--- a/Users/Users.pm
+++ b/Users/Users.pm
@@ -439,32 +439,32 @@ sub get
 sub del_user
 {
 	my ($user) = @_;
-  my ($command);
+  my (@command);
 	
   if ($Utils::Backend::tool{"system"} eq "FreeBSD")
   {
-    $command = "$cmd_pw userdel -n \'" . $$user[$LOGIN] . "\' ";
+    @command = ($cmd_pw, "userdel", "-n", $$user[$LOGIN]);
   }
   else
   {
     if ($cmd_deluser)
     {
-      $command = "$cmd_deluser '". $$user[$LOGIN] . "'";
+      @command = ($cmd_deluser, $$user[$LOGIN]);
     }
     else
     {
-      $command = "$cmd_userdel \'" . $$user[$LOGIN] . "\'";
+      @command = ($cmd_userdel, $$user[$LOGIN]);
     }
   }
 
-  &Utils::File::run ($command);
+  &Utils::File::run (@command);
 }
 
 sub change_user_chfn
 {
   my ($login, $old_comment, $comment) = @_;
   my ($fname, $office, $office_phone, $home_phone);
-  my ($command, $str);
+  my (@command, $str);
 
   return if !$login;
 
@@ -474,14 +474,15 @@ sub change_user_chfn
 
   if ($Utils::Backend::tool{"system"} eq "FreeBSD")
   {
-    $command = "$cmd_pw usermod -n " . $login . " -c \'" . $str . "\'";
+    @command = ($cmd_pw, "usermod", "-n", $login,
+                                    "-c", $str);
   }
   else
   {
-    $command = "$cmd_usermod -c \'" . $str . "\' " . $login;
+    @command = ($cmd_usermod, "-c", $str, $login);
   }
 
-  &Utils::File::run ($command);
+  &Utils::File::run (@command);
 }
 
 # modifies /etc/shadow directly, not good practice,
@@ -515,11 +516,11 @@ sub modify_shadow_password
 sub set_passwd
 {
   my ($login, $password) = @_;
-  my ($pwdpipe, $command);
+  my ($pwdpipe);
 
   if ($Utils::Backend::tool{"system"} eq "FreeBSD")
   {
-
+    my ($command);
     $command = "$cmd_pw usermod -H 0";
     $pwdpipe = &Utils::File::run_pipe_write ($command);
     print $pwdpipe $password;
@@ -531,10 +532,10 @@ sub set_passwd
   }
   else
   {
-    $command = "$cmd_usermod " .
-        " -p '" . $password . "' " . $login;
+    my (@command);
+    @command = ($cmd_usermod, "-p", $password, $login);
 
-    &Utils::File::run ($command);
+    &Utils::File::run (@command);
   }
 }
 
@@ -552,7 +553,7 @@ sub add_user
 
     # FreeBSD doesn't create the home directory
     $home = $$user[$HOME];
-    &Utils::File::run ("$tool_mkdir -p $home");
+    &Utils::File::run ($tool_mkdir, "-p", $home);
 
     $command = "$cmd_pw useradd " .
         " -n \'" . $$user[$LOGIN] . "\'" .
@@ -562,6 +563,13 @@ sub add_user
         " -s \'" . $$user[$SHELL] . "\'" .
         " -H 0"; # pw(8) reads password from STDIN
 
+#    @command = ($cmd_pw, "useradd", "-n", $$user[$LOGIN],
+#                                    "-u", $$user[$UID],
+#                                    "-d", $$user[$HOME],
+#                                    "-g", $$user[$GID],
+#                                    "-s", $$user[$SHELL],
+#                                    "-H", "0"); # pw(8) reads password from STDIN
+
     $pwdpipe = &Utils::File::run_pipe_write ($command);
     print $pwdpipe $$user[$PASSWD];
     &Utils::File::close_file ($pwdpipe);
@@ -570,23 +578,22 @@ sub add_user
   {
     $home_parents = $$user[$HOME];
     $home_parents =~ s/\/+[^\/]+\/*$//;
-    &Utils::File::run ("$tool_mkdir -p $home_parents");
+    &Utils::File::run ($tool_mkdir, "-p", $home_parents);
 
-    $command = "$cmd_useradd" .
-        " -d \'" . $$user[$HOME]  . "\'" .
-        " -g \'" . $$user[$GID]   . "\'" .
-        " -s \'" . $$user[$SHELL] . "\'" .
-        " -u \'" . $$user[$UID]   . "\'" .
-        " \'"    . $$user[$LOGIN] . "\'";
+    @command = ($cmd_useradd, "-d", $$user[$HOME],
+                              "-g", $$user[$GID],
+                              "-s", $$user[$SHELL],
+                              "-u", $$user[$UID],
+                                    $$user[$LOGIN]);
 
-    &Utils::File::run ($command);
+    &Utils::File::run (@command);
     &modify_shadow_password ($$user[$LOGIN], $$user[$PASSWD]);
   }
   else
   {
     $home_parents = $$user[$HOME];
     $home_parents =~ s/\/+[^\/]+\/*$//;
-    &Utils::File::run ("$tool_mkdir -p $home_parents");
+    &Utils::File::run ($tool_mkdir, "-p", $home_parents);
 
     if ($cmd_adduser &&
         $Utils::Backend::tool{"platform"} !~ /^slackware/ &&
@@ -596,34 +603,34 @@ sub add_user
     {
       # use adduser if available and valid (slackware one is b0rk)
       # set empty gecos fields and password, they will be filled out later
-      $command = "$cmd_adduser --gecos '' --disabled-password" .
-          " --home \'"  . $$user[$HOME]   . "\'" .
-          " --gid \'"   . $$user[$GID]    . "\'" .
-          " --shell \'" . $$user[$SHELL]  . "\'" .
-          " --uid \'"   . $$user[$UID]    . "\'" .
-          " \'"         . $$user[$LOGIN]  . "\'";
+      @command = ($cmd_adduser, "--gecos", "",
+                                "--disabled-password",
+                                "--home", $$user[$HOME],
+                                "--gid", $$user[$GID],
+                                "--shell", $$user[$SHELL],
+                                "--uid", $$user[$UID],
+                                         $$user[$LOGIN]);
 
-      &Utils::File::run ($command);
+      &Utils::File::run (@command);
 
       # password can't be set in non-interactive
       # mode with adduser, call usermod instead
-      $command = "$cmd_usermod " .
-          " -p '" . $$user[$PASSWD] . "' " . $$user[$LOGIN];
+      @command = ($cmd_usermod, "-p", $$user[$PASSWD], $$user[$LOGIN]);
 
-      &Utils::File::run ($command);
+      &Utils::File::run (@command);
     }
     else
     {
       # fallback to useradd
-      $command = "$cmd_useradd -m" .
-          " -d \'" . $$user[$HOME]   . "\'" .
-          " -g \'" . $$user[$GID]    . "\'" .
-          " -p \'" . $$user[$PASSWD] . "\'" .
-          " -s \'" . $$user[$SHELL]  . "\'" .
-          " -u \'" . $$user[$UID]    . "\'" .
-          " \'"    . $$user[$LOGIN]  . "\'";
-
-      &Utils::File::run ($command);
+      @command = ($cmd_useradd, "-m",
+                                "-d", $$user[$HOME],
+                                "-g", $$user[$GID],
+                                "-p", $$user[$PASSWD],
+                                "-s", $$user[$SHELL],
+                                "-u", $$user[$UID],
+                                      $$user[$LOGIN]);
+
+      &Utils::File::run (@command);
     }
   }
 
@@ -652,27 +659,24 @@ sub change_user
   }
   elsif ($Utils::Backend::tool{"system"} eq "SunOS")
   {
-    $command = "$cmd_usermod" .
-        " -d \'" . $$new_user[$HOME]   . "\'" .
-        " -g \'" . $$new_user[$GID]    . "\'" .
-        " -l \'" . $$new_user[$LOGIN]  . "\'" .
-        " -s \'" . $$new_user[$SHELL]  . "\'" .
-        " -u \'" . $$new_user[$UID]    . "\'" .
-        " \'" . $$old_user[$LOGIN] . "\'";
+    @command = ($cmd_usermod, "-d", $$new_user[$HOME],
+                              "-g", $$new_user[$GID],
+                              "-l", $$new_user[$LOGIN],
+                              "-s", $$new_user[$SHELL],
+                              "-u", $$new_user[$UID],
+                                    $$old_user[$LOGIN]);
 
-    &Utils::File::run ($command);
+    &Utils::File::run (@command);
     &modify_shadow_password ($$new_user[$LOGIN], $$new_user[$PASSWD]);
   }
   else
   {
-    $command = "$cmd_usermod" .
-        " -d \'" . $$new_user[$HOME]   . "\'" .
-        " -g \'" . $$new_user[$GID]    . "\'" .
-        " -l \'" . $$new_user[$LOGIN]  . "\'" .
-        " -p \'" . $$new_user[$PASSWD] . "\'" .
-        " -s \'" . $$new_user[$SHELL]  . "\'" .
-        " -u \'" . $$new_user[$UID]    . "\'" .
-        " \'" . $$old_user[$LOGIN] . "\'";
+    @command = ($cmd_usermod, "-d", $$new_user[$HOME],
+                              "-g", $$new_user[$GID],
+                              "-l", $$new_user[$LOGIN],
+                              "-s", $$new_user[$SHELL],
+                              "-u", $$new_user[$UID],
+                                    $$old_user[$LOGIN]);
 
     &Utils::File::run ($command);
   }
diff --git a/Utils/File.pm b/Utils/File.pm
index 6cdd8a6..0bace55 100644
--- a/Utils/File.pm
+++ b/Utils/File.pm
@@ -55,34 +55,18 @@ sub get_backup_path
   return (&get_base_path () . "/backup");
 }
 
-# Give a command, and it will put in C locale, some sane PATH values and find
+# Give a command, and it will put in C locale and find
 # the program to run in the path. Redirects stderr to null.
-sub do_get_cmd_path
-{
-  my ($cmd) = @_;
-  my ($tool_name, @argline, $command, $tool_path);
-  
-  ($tool_name, @argline) = split("[ \t]+", $cmd);
-
-  $tool_path = &locate_tool ($tool_name);
-  return -1 if ($tool_path eq "");
-
-  $command = "$tool_path @argline";
-  # Do not escape args, it's reasonable
-  # to assume they're already escaped
-  #$command =~ s/\"/\\\"/g;
-
-  return $command;
-}
-
 sub get_cmd_path
 {
    my ($cmd) = @_;
 
-   my $command = &do_get_cmd_path ($cmd);
+   my ($tool_name, @argline) = split("[ \t]+", $cmd);
+   my $tool_path = &locate_tool ($tool_name);
+   return -1 if ($tool_path eq "");
 
-   return -1 if ($command == -1);
-   return ("LC_ALL=C PATH=\$PATH:/sbin:/usr/sbin $command 2> /dev/null");
+   $command = "$tool_path @argline";
+   return ("LC_ALL=C $command 2> /dev/null");
 }
 
 # necessary for some programs that output info through stderr
@@ -91,7 +75,7 @@ sub get_cmd_path_with_stderr
    my ($cmd) = @_;
 
    my $command = &get_cmd_path ($cmd);
-   return ("LC_ALL=C PATH=\$PATH:/sbin:/usr/sbin $command 2>&1");
+   return ("LC_ALL=C $command 2>&1");
 }
 
 
@@ -460,7 +444,7 @@ sub run_pipe
   $command .= " |" if $mode_mask & $FILE_READ;
   $command = "| $command > /dev/null" if $mode_mask & $FILE_WRITE;
 
-  open PIPE, $command;
+  open *PIPE, $command;
   &Utils::Report::do_report ("file_run_pipe_success", $command);
   &Utils::Report::leave ();
   return *PIPE;
@@ -764,40 +748,66 @@ sub read_joined_lines
 # --- Command-line utilities --- #
 
 
-# &run (<command line>)
+# &run_full (<in background>, <command>, <array of arguments>)
 #
-# Assumes the first word on the command line is the command-line utility
+# Takes a boolean indicating whether to run the program in the background,
+# an array containing a command to run and the arguments to pass.
+# Assumes the first word in the array is the command-line utility
 # to run, and tries to locate it, replacing it with its full path. The path
 # is cached in a hash, to avoid searching for it repeatedly. Output
 # redirection is appended, to make the utility perfectly silent. The
-# preprocessed command line is run, and its exit value is returned.
+# preprocessed command line is run, and its exit value is returned,
+# or -1 on failure. When run in the background, 0 is returned if
+# the fork did succeed, disregarding possible failure of exec().
 #
-# Example: "mkswap /dev/hda3" -> 'PATH=$PATH:/sbin:/usr/sbin /sbin/mkswap /dev/hda3 2>/dev/null >/dev/null'.
 
-sub run
+sub run_full
 {
-  my ($cmd, $background) = @_;
-  my ($command, $tool_name, $tool_path, @argline);
+  my ($background, $cmd, @arguments) = @_;
+  my ($command, $tool_name, $tool_path, $pid);
 
   &Utils::Report::enter ();
 
-  $command = &get_cmd_path ($cmd);
+  $tool_path = &locate_tool ($cmd);
+  return -1 if ($tool_path eq "");
   return -1 if $command == -1;
-  $command .= " > /dev/null";
-  $command .= " &" if $background;
 
-  &Utils::Report::do_report ("file_run", $command);
+  $command = join (" ", ($tool_path, @arguments));
+  &Utils::Report::do_report ("file_run_full", $command);
   &Utils::Report::leave ();
 
+  my $pid = fork();
+
+  return -1 if (!defined $pid);
+
+  if ($pid == 0)
+  {
+    $ENV{"LC_ALL"} = "C";
+    open (STDOUT, "/dev/null");
+    open (STDERR, "/dev/null");
+    exec ($tool_path, @arguments);
+    exit (0);
+  }
+
+  # If no error has occurred so far, assume success,
+  # ignoring the future return value
+  return 0 if ($background);
+
+  waitpid ($pid, 0);
+
   # As documented in perlfunc, divide by 256.
-  return (system ($command) / 256);
+  return ($? / 256);
 }
 
+# Simple wrappers calling &run_full() with the right background parameter
 sub run_bg
 {
-  my ($cmd) = @_;
+  return &run_full (1, @_);
+}
 
-  return &run ($cmd, 1);
+sub run
+{
+  return &run_full (0, @_);
 }
 
 # &gst_file_locate_tool
diff --git a/Utils/Report.pm b/Utils/Report.pm
index 7800e4c..6cb9271 100644
--- a/Utils/Report.pm
+++ b/Utils/Report.pm
@@ -188,7 +188,7 @@ sub add
      "file_open_write_success"  => ["info",  "Writing to [%s]."],
      "file_run_pipe_failed"     => ["warn",  "Failed to pipe command [%s] for reading."],
      "file_run_pipe_success"    => ["info",  "Piping command [%s] for reading."],
-     "file_run"                 => ["info",  "Running command [%s]."],
+     "file_run_full"            => ["info",  "Running command [%s]."],
      "file_create_path"         => ["info",  "Directory [%s] created."],
      "file_backup_rotate"       => ["info",  "Backup directory [%s] was rotated."],
      "file_backup_success"      => ["info",  "Saved backup for [%s]."],



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