On Friday 14 November 2014 10:11:34 Tor Arntsen wrote: > On 13 November 2014 15:45, Peter Wu <[email protected]> wrote: > > Heya, > > > > Another round of patches around the test suite. This time the patches > > fix tests that run under LD_PRELOAD=libhostname.so and when cURL is > > built with Address Sanitizer (-fsanitize=address). > > > > The first patch ("runtests.pl: simplify command environment handling") > > actually contains three changes: > [..] > > I would like to check out the patches on my non-Linux build systems. > Could you re-send the patches as attachments please? The best practice > is probably to both include as well as attach the diffs. > > (The included diffs look fine, it's just that Google (I use gmail to > read list material due to the high traffic) in their infinite wisdown > changed the user interface so that it's no longer possible to actually > save an email to file (it's nearly impossible to copy and paste even, > and never accurately). Something that worked fine before. But as > usual, each and every gmail update downgrades the user interface to > even closer to useless.)
You can find the patches in raw form (with '@' mangled in addresses) at http://curl.haxx.se/mail/lib-2014-11/0213.html I also push out changes to https://github.com/Lekensteyn/curl/compare/master...fix-cmake Note that I also introduced two bugs with the Text::ParseWords and IPC::Run replacements. The Text::ParseWords implementation has an issue which is fixed below (incorporated in the attached patch): diff --git a/tests/runtests.pl b/tests/runtests.pl index 265b9f8..b465ae9 100755 --- a/tests/runtests.pl +++ b/tests/runtests.pl @@ -563,5 +563,5 @@ sub cmd_tostring { # Splits a shell command into an array that can be passed to exec. sub splitcmd { - shift; + my $cmdline = shift; my @args; my $arg; @@ -569,5 +569,5 @@ sub splitcmd { # Text::ParseWords is broken in this regards as it converts "\{\}\n" to # "{}n" instead of keeping the backslashes here. - while (/ + while ($cmdline =~ / # Double quote (")((?:[^\\"]+|\\.)*)"| The other bug is in the redirection of stdout/stderr to a variable. The output is redirected to a scalar reference... but since the process has forked, there is no way that this output is available to the caller. I am working on a fix for this. Hopefully these bugs did not waste your time. For reference, this is my test output (runtests.pl -n -a -k) with these patches: TESTDONE: 489 tests out of 489 reported OK: 100% TESTDONE: 955 tests were considered during 398 seconds. TESTINFO: 466 tests were skipped due to these restraints: TESTINFO: "curl lacks debug support" 76 times (67, 68, 69, 81, 89, 90, 91, 150, 155 and 67 more) TESTINFO: "failed starting SMTP server" 38 times (900, 901, 902, 903, 904, 905, 908, 909, 910 and 29 more) TESTINFO: "failed starting TFTP server" 13 times (271, 283, 284, 285, 286, 1007, 1009, 1049, 1093 and 4 more) TESTINFO: "failed starting IMAP server" 33 times (800, 801, 802, 803, 804, 805, 806, 807, 808 and 24 more) TESTINFO: "configured as DISABLED" 8 times (594, 836, 882, 938, 1209, 1211, 1316, 1512) TESTINFO: "curl lacks TrackMemory support" 2 times (96, 558) TESTINFO: "curl lacks large_file support" 3 times (99, 1044, 1063) TESTINFO: "no stunnel" 28 times (300, 301, 302, 303, 304, 305, 306, 307, 308 and 19 more) TESTINFO: "failed starting HTTP-pipe server" 4 times (1900, 1901, 1902, 1903) TESTINFO: "failed starting FTP-IPv6 server" 6 times (252, 253, 254, 255, 1048, 1050) TESTINFO: "curl lacks Metalink support" 16 times (2005, 2008, 2009, 2010, 2011, 2012, 2013, 2014, 2015 and 7 more) TESTINFO: "curl lacks unittest support" 16 times (1300, 1301, 1302, 1303, 1304, 1305, 1306, 1307, 1308 and 7 more) TESTINFO: "curl lacks TLS-SRP support" 5 times (320, 321, 322, 323, 324) TESTINFO: "failed starting POP3 server" 29 times (850, 851, 852, 853, 854, 855, 856, 857, 858 and 20 more) TESTINFO: "precheck command error" 2 times (241, 1083) TESTINFO: "curl lacks NTLM_WB support" 1 times (1310) TESTINFO: "failed starting FTP server" 186 times (100, 101, 102, 103, 104, 105, 106, 107, 108 and 177 more) -- Kind regards, Peter https://lekensteyn.nl
>From 7d3837377b03532f943d116160229d0be2d783b8 Mon Sep 17 00:00:00 2001 From: Peter Wu <[email protected]> Date: Sun, 2 Nov 2014 10:52:10 +0100 Subject: [PATCH 2/2] runtests.pl: simplify command environment handling Rather than saving valid environment variables for restoration later, use 'local' to limit the environment changes to the block executing a command. Rationale: LD_PRELOAD may not work with all executables. Previously it would break on the shell when compiled with AddressSanitizer (-fsanitize=address), but that is avoided by not executing string commands anymore. gdb would still be executed with LD_PRELOAD though, so setting $ENV in this way was not sustainable. After this patch, environment variables are passed to executions of precheck (for chkhostname), command (for curl) and postcheck (for symmetry with precheck, it does not actually use it). Note that this patch does not help with an autotools build which uses shared libs and AddressSanitizer as the 'curl' program is a libtool wrapper (shell script). Signed-off-by: Peter Wu <[email protected]> --- tests/runtests.pl | 66 ++++++++++++++++++++++--------------------------------- 1 file changed, 26 insertions(+), 40 deletions(-) diff --git a/tests/runtests.pl b/tests/runtests.pl index b465ae9..522b079 100755 --- a/tests/runtests.pl +++ b/tests/runtests.pl @@ -266,7 +266,6 @@ my %timesrvrlog; # timestamp for each test server logs lock removal my %timevrfyend; # timestamp for each test result verification end my $testnumcheck; # test number, set in singletest sub. -my %oldenv; ####################################################################### # variables that command line options may set @@ -509,6 +508,7 @@ sub checktestcmd { # - stdout and stderr are either strings (a filename or '&1' for stdout) or # a reference to store the output. Note: stdout is always redirected before # stderr. +# - env is an optional hash which sets env vars for the child process. # sub runclient { my %args = @_; @@ -533,6 +533,8 @@ sub runclient { } } } + # Export env vars and prevent leaking vars such as LD_PRELOAD to sh. + local %ENV = (%ENV, %{$args{env}}) if $args{env}; # Try to invoke the command or exit exec(@{$args{cmd}}) or exit(127); } @@ -3123,17 +3125,6 @@ sub singletest { # this is done this early, so that the precheck can use environment # variables and still bail out fine on errors - # restore environment variables that were modified in a previous run - foreach my $var (keys %oldenv) { - if($oldenv{$var} eq 'notset') { - delete $ENV{$var} if($ENV{$var}); - } - else { - $ENV{$var} = $oldenv{$var}; - } - delete $oldenv{$var}; - } - # remove test server commands file before servers are started/verified unlink($FTPDCMD) if(-f $FTPDCMD); @@ -3148,31 +3139,29 @@ sub singletest { $timesrvrend{$testnum} = Time::HiRes::time() if($timestats); my @setenv = getpart("client", "setenv"); + # Environment variables for precheck, command and postcheck. Since + # LD_PRELOAD may not be able to load in all programs (for example, when + # compiling with AddressSanitizer enabled), only apply it to programs built + # in the source tree, and not perl, sh, etc. + my %env; if(@setenv) { foreach my $s (@setenv) { chomp $s; subVariables \$s; if($s =~ /([^=]*)=(.*)/) { my ($var, $content) = ($1, $2); - # remember current setting, to restore it once test runs - $oldenv{$var} = ($ENV{$var})?"$ENV{$var}":'notset'; # set new value - if(!$content) { - delete $ENV{$var} if($ENV{$var}); - } - else { - if($var =~ /^LD_PRELOAD/) { - if(exe_ext() && (exe_ext() eq '.exe')) { - # print "Skipping LD_PRELOAD due to lack of OS support\n"; - next; - } - if($debug_build || ($has_shared ne "yes")) { - # print "Skipping LD_PRELOAD due to no release shared build\n"; - next; - } + if($var =~ /^LD_PRELOAD/) { + if(exe_ext() && (exe_ext() eq '.exe')) { + # print "Skipping LD_PRELOAD due to lack of OS support\n"; + next; + } + if($debug_build || ($has_shared ne "yes")) { + # print "Skipping LD_PRELOAD due to no release shared build\n"; + next; } - $ENV{$var} = "$content"; } + $env{$var} = "$content"; } } } @@ -3206,6 +3195,7 @@ sub singletest { cmd => \@p, stderr => File::Spec->devnull, stdout => \$out, + env => \%env, ); my $rc = runclient(%cmd); $why = (split(/\n/, $out))[0]; @@ -3375,6 +3365,7 @@ sub singletest { my %test_command = ( stdout => $STDOUT, stderr => $STDERR, + env => \%env, ); my $CMDLINE; my $cmdargs; @@ -3515,6 +3506,12 @@ sub singletest { my $gdbinit = "$TESTDIR/gdbinit$testnum"; open(GDBCMD, ">$LOGDIR/gdbcmd"); print GDBCMD "set args $cmdargs\n"; + keys %env; + while (my ($k, $v) = each %env) { + print GDBCMD "set environment $k = $v\n"; + } + # Do not invoke using a shell as LD_PRELOAD may crash it. + print GDBCMD "set startup-with-shell off\n"; print GDBCMD "show args\n"; print GDBCMD "source $gdbinit\n" if -e $gdbinit; close(GDBCMD); @@ -3704,6 +3701,7 @@ sub singletest { my @cmda = splitcmd("$cmd"); my $rc = runclient( cmd => \@cmda, + env => \%env, ); # Must run the postcheck command in torture mode in order # to clean up, but the result can't be relied upon. @@ -3716,18 +3714,6 @@ sub singletest { } } - # restore environment variables that were modified - if(%oldenv) { - foreach my $var (keys %oldenv) { - if($oldenv{$var} eq 'notset') { - delete $ENV{$var} if($ENV{$var}); - } - else { - $ENV{$var} = "$oldenv{$var}"; - } - } - } - # Skip all the verification on torture tests if ($torture) { if(!$cmdres && !$keepoutfiles) { -- 2.1.2
>From b15215ac154065b96eebfd81862aa3cc108a167a Mon Sep 17 00:00:00 2001 From: Peter Wu <[email protected]> Date: Sat, 1 Nov 2014 22:44:04 +0100 Subject: [PATCH 1/2] runtests.pl: safer system() calls, fix LD_PRELOAD Command invocation via runclient() (which calls system()) using a string invokes the shell. This has the problem that tests which load libhostname.so via LD_PRELOAD break when the library cannot be loaded into the shell (when cURL is built with AddressSanitizer for example). This patch removes the ability to use shell constructs in commands, such as backticks, but fixes the above case where an ASAN-enabled library is loaded with LD_PRELOAD. The system call (in runclient) and backticks operator (for precheck) are replaced by fork/open/exec to avoid executing sh. Note that sh is still invoked when curl is built with shared libs in autotools due to the libtool wrapper script. Note for the curious: previously a gdb invocation would feed stdin via 'set args <input'. This only works because the command is interpreted as a shell command. After this patch stdin is fed directly. Detection of the `valgrind --tool` option was previously done by invoking grep. Remove this use of grep. While at it, remove `valgrind --version` invocation to determine whether `--log-file` should be used instead of `--logfile`. Parse this information directly from `valgrind --help` which was executed earlier. As for differences in log file, these are just whitespace (trailing space for `curl --version` command, spaces after redirection operator) and quoting (quotes typically get stripped). Signed-off-by: Peter Wu <[email protected]> --- tests/runtests.pl | 380 +++++++++++++++++++++++++++++++++++++----------------- 1 file changed, 261 insertions(+), 119 deletions(-) diff --git a/tests/runtests.pl b/tests/runtests.pl index 8802c0c..b465ae9 100755 --- a/tests/runtests.pl +++ b/tests/runtests.pl @@ -69,6 +69,7 @@ BEGIN { use strict; use warnings; use Cwd; +use File::Spec; # Subs imported from serverhelp module use serverhelp qw( @@ -502,45 +503,111 @@ sub checktestcmd { ####################################################################### # Run the application under test and return its return code # +# A hash is expected as parameter. Keys: +# - cmd is a reference to an array. +# - stdin is an optional filename. +# - stdout and stderr are either strings (a filename or '&1' for stdout) or +# a reference to store the output. Note: stdout is always redirected before +# stderr. +# sub runclient { - my ($cmd)=@_; - my $ret = system($cmd); - print "CMD ($ret): $cmd\n" if($verbose && !$torture); - return $ret; + my %args = @_; + # To test curl on a remote machine, replace the command by + # 'ssh $CLIENTIP "cd $PWD && $cmd', but note shell escaping and take care of + # env vars. Then sleep(2) to allow the NFS server to be updated. + my $pid = fork(); + if (!defined($pid)) { + print "fork failed with $@\n" if($verbose && !$torture); + return -1; + } + if ($pid == 0) { # child + open(STDIN, '<', $args{stdin}) if $args{stdin}; + my %outputs = ( 'stdout' => *STDOUT, 'stderr' => *STDERR ); + while (my ($key, $handle) = each %outputs) { + if (my $dest = $args{$key}) { + if ($dest =~ /^&/) { + open($handle, ">$dest"); + } else { + # Scalar or other file name (do not treat "-" specially!) + open($handle, '>', $dest); + } + } + } + # Try to invoke the command or exit + exec(@{$args{cmd}}) or exit(127); + } -# This is one way to test curl on a remote machine -# my $out = system("ssh $CLIENTIP cd \'$pwd\' \\; \'$cmd\'"); -# sleep 2; # time to allow the NFS server to be updated -# return $out; + # Parent + waitpid($pid, 0); + my $ret = $?; + print "CMD ($ret): " . cmd_tostring(%args) . "\n" if($verbose && !$torture); + return $ret; } ####################################################################### -# Run the application under test and return its stdout +# Converts the options (that can be passed to runclient) to a string. # -sub runclientoutput { - my ($cmd)=@_; - return `$cmd`; +sub cmd_tostring { + my %args = @_; + my @cmd; + push @cmd, "'" . s/'/'\\''/gr . "'" for (@{$args{cmd}}); + # when given, stdin can only be a filename. + push @cmd, "<$args{stdin}" if $args{stdin}; + # Print redir op only if redirection is redirected which is a filename (and + # not a reference to an output variable). + push @cmd, ">$args{stdout}" if ($args{stdout} and !ref($args{stdout})); + push @cmd, "2>$args{stderr}" if ($args{stderr} and !ref($args{stderr})); + return "@cmd"; +} -# This is one way to test curl on a remote machine -# my @out = `ssh $CLIENTIP cd \'$pwd\' \\; \'$cmd\'`; -# sleep 2; # time to allow the NFS server to be updated -# return @out; - } +# Splits a shell command into an array that can be passed to exec. +sub splitcmd { + my $cmdline = shift; + my @args; + my $arg; + # Match all arguments, removing quotes/backslashes as needed. + # Text::ParseWords is broken in this regards as it converts "\{\}\n" to + # "{}n" instead of keeping the backslashes here. + while ($cmdline =~ / + # Double quote + (")((?:[^\\"]+|\\.)*)"| + # Single quotes + (')([^']*)'| + # Whitespace + ([ \t]+)| + # Unquoted + ([^'"\\ \t]+|\\.) + /xsg) { + if (defined($1)) { # Double quotes + # IEEE Std 1003.1 2004: only touch $ ` " \ <newline> + $arg .= $+ =~ s/\\([\$\`"\\\n])/$1/gr; + } elsif (defined($3)) { # Single quotes + $arg .= $+; + } elsif (defined($5)) { # Whitespace + push @args, $arg if defined($arg); + $arg = undef; + } else { # Unquoted + $arg .= $+ =~ s/\\(.)/$1/gr; + } + } + push @args, $arg if defined($arg); + return @args; +} ####################################################################### # Memory allocation test and failure torture testing. # sub torture { - my $testcmd = shift; - my $gdbline = shift; + my %testcmd = %{shift()}; + my %gdbline = %{shift()}; # remove memdump first to be sure we get a new nice and clean one unlink($memdump); # First get URL from test server, ignore the output/result - runclient($testcmd); + runclient(%testcmd); - logmsg " CMD: $testcmd\n" if($verbose); + logmsg " CMD: " . cmd_tostring(%testcmd) . "\n" if($verbose); # memanalyze -v is our friend, get the number of allocations made my $count=0; @@ -584,10 +651,10 @@ sub torture { my $ret = 0; if($gdbthis) { - runclient($gdbline) + runclient(%gdbline) } else { - $ret = runclient($testcmd); + $ret = runclient(%testcmd); } #logmsg "$_ Returned " . ($ret >> 8) . "\n"; @@ -723,20 +790,24 @@ sub verifyhttp { $bonus="1/"; } - my $flags = "--max-time $server_response_maxtime "; - $flags .= "--output $verifyout "; - $flags .= "--silent "; - $flags .= "--verbose "; - $flags .= "--globoff "; - $flags .= "-1 " if($has_axtls); - $flags .= "--insecure " if($proto eq 'https'); - $flags .= "\"$proto://$ip:$port/${bonus}verifiedserver\""; - - my $cmd = "$VCURL $flags 2>$verifylog"; + my %cmd = ( + cmd => [ + $VCURL, + "--max-time", $server_response_maxtime, + "--output", $verifyout, + "--silent", + "--verbose", + "--globoff", + $has_axtls ? "-1" : (), + $proto eq 'https' ? "--insecure" : (), + "$proto://$ip:$port/${bonus}verifiedserver", + ], + stderr => $verifylog, + ); # verify if our/any server is running on this port - logmsg "RUN: $cmd\n" if($verbose); - my $res = runclient($cmd); + logmsg "RUN: " . cmd_tostring(%cmd) . "\n" if($verbose); + my $res = runclient(%cmd); $res >>= 8; # rotate the result if($res & 128) { @@ -789,43 +860,41 @@ sub verifyftp { my $server = servername_id($proto, $ipvnum, $idnum); my $pid = 0; my $time=time(); - my $extra=""; my $verifylog = "$LOGDIR/". servername_canon($proto, $ipvnum, $idnum) .'_verify.log'; unlink($verifylog) if(-f $verifylog); - if($proto eq "ftps") { - $extra .= "--insecure --ftp-ssl-control "; - } - - my $flags = "--max-time $server_response_maxtime "; - $flags .= "--silent "; - $flags .= "--verbose "; - $flags .= "--globoff "; - $flags .= $extra; - $flags .= "\"$proto://$ip:$port/verifiedserver\""; - - my $cmd = "$VCURL $flags 2>$verifylog"; + my $data = ''; + my %cmd = ( + cmd => [ + $VCURL, + "--max-time", $server_response_maxtime, + "--silent", + "--verbose", + "--globoff", + $proto eq "ftps" ? ("--insecure", "--ftp-ssl-control") : (), + "$proto://$ip:$port/verifiedserver", + ], + stderr => $verifylog, + stdout => \$data, + ); # check if this is our server running on this port: - logmsg "RUN: $cmd\n" if($verbose); - my @data = runclientoutput($cmd); + logmsg "RUN: " . cmd_tostring(%cmd) . "\n" if($verbose); + my $res = runclient(%cmd); - my $res = $? >> 8; # rotate the result + $res >>= 8; # rotate the result if($res & 128) { logmsg "RUN: curl command died with a coredump\n"; return -1; } - foreach my $line (@data) { - if($line =~ /WE ROOLZ: (\d+)/) { - # this is our test server with a known pid! - $pid = 0+$1; - last; - } + if($data =~ /WE ROOLZ: (\d+)/) { + # this is our test server with a known pid! + $pid = 0+$1; } - if($pid <= 0 && @data && $data[0]) { + if($pid <= 0 && $data) { # this is not a known server logmsg "RUN: Unknown server on our $server port: $port\n"; return 0; @@ -861,19 +930,23 @@ sub verifyrtsp { servername_canon($proto, $ipvnum, $idnum) .'_verify.log'; unlink($verifylog) if(-f $verifylog); - my $flags = "--max-time $server_response_maxtime "; - $flags .= "--output $verifyout "; - $flags .= "--silent "; - $flags .= "--verbose "; - $flags .= "--globoff "; - # currently verification is done using http - $flags .= "\"http://$ip:$port/verifiedserver\"";; - - my $cmd = "$VCURL $flags 2>$verifylog"; + my %cmd = ( + cmd => [ + $VCURL, + "--max-time", $server_response_maxtime, + "--output", $verifyout, + "--silent", + "--verbose", + "--globoff", + # currently verification is done using http + "http://$ip:$port/verifiedserver";, + ], + stderr => $verifylog, + ); # verify if our/any server is running on this port - logmsg "RUN: $cmd\n" if($verbose); - my $res = runclient($cmd); + logmsg "RUN: " . cmd_tostring(%cmd) . "\n" if($verbose); + my $res = runclient(%cmd); $res >>= 8; # rotate the result if($res & 128) { @@ -964,8 +1037,18 @@ sub verifysftp { } # Connect to sftp server, authenticate and run a remote pwd # command using our generated configuration and key files - my $cmd = "$sftp -b $sftpcmds -F $sftpconfig -S $ssh $ip > $sftplog 2>&1"; - my $res = runclient($cmd); + my @cmd = ( + $sftp, + "-b", $sftpcmds, + "-F", $sftpconfig, + "-S", $ssh, + $ip, + ); + my $res = runclient( + cmd => \@cmd, + stdout => $sftplog, + stderr => '&1', + ); # Search for pwd command response in log file if(open(SFTPLOGFILE, "<$sftplog")) { while(<SFTPLOGFILE>) { @@ -999,21 +1082,25 @@ sub verifyhttptls { servername_canon($proto, $ipvnum, $idnum) .'_verify.log'; unlink($verifylog) if(-f $verifylog); - my $flags = "--max-time $server_response_maxtime "; - $flags .= "--output $verifyout "; - $flags .= "--verbose "; - $flags .= "--globoff "; - $flags .= "--insecure "; - $flags .= "--tlsauthtype SRP "; - $flags .= "--tlsuser jsmith "; - $flags .= "--tlspassword abc "; - $flags .= "\"https://$ip:$port/verifiedserver\"";; - - my $cmd = "$VCURL $flags 2>$verifylog"; + my %cmd = ( + cmd => [ + $VCURL, + "--max-time", $server_response_maxtime, + "--output", $verifyout, + "--verbose", + "--globoff", + "--insecure", + "--tlsauthtype", "SRP", + "--tlsuser", "jsmith", + "--tlspassword", "abc", + "https://$ip:$port/verifiedserver";, + ], + stderr => $verifylog, + ); # verify if our/any server is running on this port - logmsg "RUN: $cmd\n" if($verbose); - my $res = runclient($cmd); + logmsg "RUN: " . cmd_tostring(%cmd) . "\n" if($verbose); + my $res = runclient(%cmd); $res >>= 8; # rotate the result if($res & 128) { @@ -2253,12 +2340,19 @@ sub checksystem { my $curlverout="$LOGDIR/curlverout.log"; my $curlvererr="$LOGDIR/curlvererr.log"; - my $versioncmd="$CURL --version 1>$curlverout 2>$curlvererr"; + my %versioncmd = ( + cmd => [ + $CURL, + "--version", + ], + stdout => $curlverout, + stderr => $curlvererr, + ); unlink($curlverout); unlink($curlvererr); - $versretval = runclient($versioncmd); + $versretval = runclient(%versioncmd); $versnoexec = $!; open(VERSOUT, "<$curlverout"); @@ -2439,7 +2533,7 @@ sub checksystem { if(!$curl) { logmsg "unable to get curl's version, further details are:\n"; logmsg "issued command: \n"; - logmsg "$versioncmd \n"; + logmsg cmd_tostring(%versioncmd) . "\n"; if ($versretval == -1) { logmsg "command failed with: \n"; logmsg "$versnoexec \n"; @@ -2495,8 +2589,15 @@ sub checksystem { $has_shared = `sh $CURLCONFIG --built-shared`; chomp $has_shared; - my $hostname=join(' ', runclientoutput("hostname")); - my $hosttype=join(' ', runclientoutput("uname -a")); + my $hostname = my $hosttype = ''; + runclient( + cmd => ['hostname'], + stdout => \$hostname, + ); + runclient( + cmd => ['uname', '-a'], + stdout => \$hosttype, + ); logmsg ("********* System characteristics ******** \n", "* $curl\n", @@ -3088,7 +3189,7 @@ sub singletest { chomp $cmd; subVariables \$cmd; if($cmd) { - my @p = split(/ /, $cmd); + my @p = splitcmd("$cmd"); if($p[0] !~ /\//) { # the first word, the command, does not contain a slash so # we will scan the "improved" PATH to find the command to @@ -3098,17 +3199,21 @@ sub singletest { if($fullp) { $p[0] = $fullp; } - $cmd = join(" ", @p); } - my @o = `$cmd 2>/dev/null`; - if($o[0]) { - $why = $o[0]; - chomp $why; - } elsif($?) { + my $out = ""; + my %cmd = ( + cmd => \@p, + stderr => File::Spec->devnull, + stdout => \$out, + ); + my $rc = runclient(%cmd); + $why = (split(/\n/, $out))[0]; + # Any output is assumed to be an error + if (!$why && $rc != 0) { $why = "precheck command error"; } - logmsg "prechecked $cmd\n" if($verbose); + logmsg "prechecked " . cmd_tostring(%cmd) . "\n" if($verbose); } } } @@ -3267,6 +3372,10 @@ sub singletest { } } + my %test_command = ( + stdout => $STDOUT, + stderr => $STDERR, + ); my $CMDLINE; my $cmdargs; my $cmdtype = $cmdhash{'type'} || "default"; @@ -3347,7 +3456,7 @@ sub singletest { writearray($stdinfile, \@stdintest); - $cmdargs .= " <$stdinfile"; + $test_command{stdin} = $stdinfile; } if(!$tool) { @@ -3370,13 +3479,17 @@ sub singletest { } } - $CMDLINE .= "$cmdargs >$STDOUT 2>$STDERR"; + $CMDLINE .= "$cmdargs"; + + # TODO: get rid of $CMDLINE and consider using gdb --args ... + my @commandline_split = splitcmd("$CMDLINE"); + $test_command{cmd} = \@commandline_split; if($verbose) { - logmsg "$CMDLINE\n"; + logmsg cmd_tostring(%test_command) . "\n"; } - print CMDLOG "$CMDLINE\n"; + print CMDLOG cmd_tostring(%test_command) . "\n"; unlink("core"); @@ -3412,16 +3525,31 @@ sub singletest { # run the command line we built if ($torture) { - $cmdres = torture($CMDLINE, - "$gdb --directory libtest $DBGCURL -x $LOGDIR/gdbcmd"); + my %gdb_command = ( + cmd => [ + $gdb, + "--directory", "libtest", + $DBGCURL, + "-x", "$LOGDIR/gdbcmd" + ], + ); + $cmdres = torture(\%test_command, \%gdb_command); } elsif($gdbthis) { my $GDBW = ($gdbxwin) ? "-w" : ""; - runclient("$gdb --directory libtest $DBGCURL $GDBW -x $LOGDIR/gdbcmd"); + runclient( + cmd => [ + $gdb, + "--directory", "libtest", + $DBGCURL, + $GDBW, + "-x", "$LOGDIR/gdbcmd", + ] + ); $cmdres=0; # makes it always continue after a debugged run } else { - $cmdres = runclient("$CMDLINE"); + $cmdres = runclient(%test_command); my $signal_num = $cmdres & 127; $dumped_core = $cmdres & 128; @@ -3451,7 +3579,16 @@ sub singletest { open(GDBCMD, ">$LOGDIR/gdbcmd2"); print GDBCMD "bt\n"; close(GDBCMD); - runclient("$gdb --directory libtest -x $LOGDIR/gdbcmd2 -batch $DBGCURL core "); + runclient( + cmd => [ + $gdb, + "--directory", "libtest", + "-x", "$LOGDIR/gdbcmd2", + "-batch", + $DBGCURL, + "core", + ] + ); # unlink("$LOGDIR/gdbcmd2"); } } @@ -3564,7 +3701,10 @@ sub singletest { subVariables \$cmd; if($cmd) { logmsg "postcheck $cmd\n" if($verbose); - my $rc = runclient("$cmd"); + my @cmda = splitcmd("$cmd"); + my $rc = runclient( + cmd => \@cmda, + ); # Must run the postcheck command in torture mode in order # to clean up, but the result can't be relied upon. if($rc != 0 && !$torture) { @@ -4750,7 +4890,11 @@ if($valgrind) { # we have found valgrind on the host, use it # verify that we can invoke it fine - my $code = runclient("valgrind >/dev/null 2>&1"); + my $code = runclient( + cmd => ['valgrind'], + stdout => File::Spec->devnull, + stderr => '2>&1', + ); if(($code>>8) != 1) { #logmsg "Valgrind failure, disable it\n"; @@ -4759,8 +4903,13 @@ if($valgrind) { # since valgrind 2.1.x, '--tool' option is mandatory # use it, if it is supported by the version installed on the system - runclient("valgrind --help 2>&1 | grep -- --tool > /dev/null 2>&1"); - if (($? >> 8)==0) { + my $help_output = ''; + runclient( + cmd => ['valgrind', '--help'], + stdout => \$help_output, + stderr => '2>&1', + ); + if ($help_output =~ /--tool/) { $valgrind_tool="--tool=memcheck"; } open(C, "<$CURL"); @@ -4772,15 +4921,8 @@ if($valgrind) { close(C); # valgrind 3 renamed the --logfile option to --log-file!!! - my $ver=join(' ', runclientoutput("valgrind --version")); - # cut off all but digits and dots - $ver =~ s/[^0-9.]//g; - - if($ver =~ /^(\d+)/) { - $ver = $1; - if($ver >= 3) { - $valgrind_logfile="--log-file"; - } + if ($help_output =~ /--log-file/) { + $valgrind_logfile = "--log-file"; } } } -- 2.1.2
------------------------------------------------------------------- List admin: http://cool.haxx.se/list/listinfo/curl-library Etiquette: http://curl.haxx.se/mail/etiquette.html
