{"thread":{"id":"3183","subject":"[PATCH] exportcommit: replace backticks with safe_pipe_capture() or system() - initial pass","startedAt":"2006-01-30T06:12:12Z","lastAt":"2006-01-30T06:12:12Z","messageCount":1,"participants":["Martin Langhoff"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"15241","messageId":"11386015323441-git-send-email-martin@catalyst.net.nz","threadId":"3183","inReplyTo":null,"subject":"[PATCH] exportcommit: replace backticks with safe_pipe_capture() or system() - initial pass","fromName":"Martin Langhoff","fromEmail":"martin@catalyst.net.nz","sentAt":"2006-01-30T06:12:12Z","receivedAt":"2006-01-30T06:12:12Z","isPatch":true,"sender":{"key":"martin@laptop.org","avatar":null},"body":"Replaced backticks with potentially troublesome unescaped input with\nsafe_pipe_capture().\n\nSigned-off-by: Martin Langhoff <martin@catalyst.net.nz>\n\n\n---\n\nNOTE: this has only seen some limited testing. Needs review. \n\n---\n\n git-cvsexportcommit.perl |   53 +++++++++++++++++++++++++++++-----------------\n 1 files changed, 33 insertions(+), 20 deletions(-)\n\ndcd2656443a39d64cc899fe1709a6bb35e89561d\ndiff --git a/git-cvsexportcommit.perl b/git-cvsexportcommit.perl\nindex d5fbee7..7b3a3d3 100755\n--- a/git-cvsexportcommit.perl\n+++ b/git-cvsexportcommit.perl\n@@ -27,7 +27,7 @@ print Dumper(@ARGV);\n # resolve target commit\n my $commit;\n $commit = pop @ARGV;\n-$commit = `git-rev-parse --verify \"$commit\"^0`;\n+$commit = safe_pipe_capture('git-rev-parse', '--verify', \"$commit^0\");\n chomp $commit;\n if ($?) {\n     die \"The commit reference $commit did not resolve!\";\n@@ -37,7 +37,7 @@ if ($?) {\n my $parent;\n if (@ARGV) {\n     $parent = pop @ARGV;\n-    $parent =  `git-rev-parse --verify \"$parent\"^0\"`;\n+    $parent =  safe_pipe_capture('git-rev-parse', '--verify', \"$parent^0\");\n     chomp $parent;\n     if ($?) {\n \tdie \"The parent reference did not resolve!\";\n@@ -45,7 +45,7 @@ if (@ARGV) {\n }\n \n # find parents from the commit itself\n-my @commit  = `git-cat-file commit $commit`;\n+my @commit  = safe_pipe_capture('git-cat-file', 'commit', $commit);\n my @parents;\n foreach my $p (@commit) {\n     if ($p =~ m/^$/) { # end of commit headers, we're done\n@@ -81,12 +81,12 @@ $opt_v && print \"Applying to CVS commit \n $? && die \"Error extracting the commit message\";\n \n my (@afiles, @dfiles, @mfiles);\n-my @files = `git-diff-tree -r $parent $commit`;\n+my @files = safe_pipe_capture('git-diff-tree', '-r', $parent, $commit);\n print @files;\n $? && die \"Error in git-diff-tree\";\n foreach my $f (@files) {\n     chomp $f;\n-    my @fields = split(m/\\s+/, $f);\n+    my @fields = split(m!\\s+!, $f);\n     if ($fields[4] eq 'A') {\n \tpush @afiles, $fields[5];\n     }\n@@ -104,18 +104,19 @@ undef @files; # don't need it anymore\n # check that the files are clean and up to date according to cvs\n my $dirty;\n foreach my $f (@afiles) {\n-    my $status = `cvs -q status \"$f\" | grep '^File: '`;\n-\n-    unless ($status =~ m/Status: Unknown$/) {\n-\t$dirty = 1;\n+    # This should return only one value\n+    my @status = grep(m/^File/,  safe_pipe_capture('cvs', '-q', 'status' ,$f));\n+    if (@status > 1) { warn 'Strange! cvs status returned more than one line?'};\n+    unless ($status[0] =~ m/Status: Unknown$/) {\n+ \t$dirty = 1;\n \twarn \"File $f is already known in your CVS checkout!\\n\";\n     }\n }\n foreach my $f (@mfiles, @dfiles) {\n     # TODO:we need to handle removed in cvs\n-    my $status = `cvs -q status \"$f\" | grep '^File: '`;\n-\n-    unless ($status =~ m/Status: Up-to-date$/) {\n+    my @status = grep(m/^File/,  safe_pipe_capture('cvs', '-q', 'status' ,$f));\n+    if (@status > 1) { warn 'Strange! cvs status returned more than one line?'};\n+    unless ($status[0] =~ m/Status: Up-to-date$/) {\n \t$dirty = 1;\n \twarn \"File $f not up to date in your CVS checkout!\\n\";\n     }\n@@ -132,18 +133,17 @@ if ($dirty) {\n \n print \"'Patching' binary files\\n\";\n \n-my @bfiles = `git-diff-tree -p $parent $commit | grep '^Binary'`;\n+my @bfiles = grep(m/^Binary/, safe_pipe_capture('git-diff-tree', '-p', $parent, $commit));\n @bfiles = map { chomp } @bfiles;\n foreach my $f (@bfiles) {\n     # check that the file in cvs matches the \"old\" file\n     # extract the file to $tmpdir and comparre with cmp\n-    my $tree = `git-rev-parse $parent^{tree} `;\n+    my $tree = safe_pipe_capture('git-rev-parse', \"$parent^{tree}\");\n     chomp $tree;\n     my $blob = `git-ls-tree $tree \"$f\" | cut -f 1 | cut -d ' ' -f 3`;\n     chomp $blob;\n     `git-cat-file blob $blob > $tmpdir/blob`;\n-    `cmp -q $f $tmpdir/blob`;\n-    if ($?) {\n+    if (system('cmp', '-q', $f, \"$tmpdir/blob\")) {\n \twarn \"Binary file $f in CVS does not match parent.\\n\";\n \t$dirty = 1;\n \tnext;\n@@ -175,7 +175,7 @@ if (($? >> 8) == 2) {\n }\n \n foreach my $f (@afiles) {\n-    `cvs add $f`;\n+    system('cvs', 'add', $f);\n     if ($?) {\n \t$dirty = 1;\n \twarn \"Failed to cvs add $f -- you may need to do it manually\";\n@@ -183,7 +183,7 @@ foreach my $f (@afiles) {\n }\n \n foreach my $f (@dfiles) {\n-    `cvs rm -f $f`;\n+    system('cvs', 'rm', '-f', $f);\n     if ($?) {\n \t$dirty = 1;\n \twarn \"Failed to cvs rm -f $f -- you may need to do it manually\";\n@@ -204,7 +204,7 @@ if ($dirtypatch) {\n \n if ($opt_c) {\n     print \"Autocommit\\n  $cmd\\n\";\n-    print `cvs commit -F .msg $commitfiles 2>&1`;\n+    print safe_pipe_capture('cvs', 'commit', '-F', '.msg', @afiles, @mfiles, @dfiles);\n     if ($?) {\n \tcleanupcvs(@files);\n \tdie \"Exiting: The commit did not succeed\";\n@@ -224,10 +224,23 @@ END\n sub cleanupcvs {\n     my @files = @_;\n     foreach my $f (@files) {\n-\t`cvs -q update -C \"$f\"`;\n+\tsystem('cvs', '-q', 'update', '-C', $f);\n \tif ($?) {\n \t    warn \"Warning! Failed to cleanup state of $f\\n\";\n \t}\n     }\n }\n \n+# An alterative to `command` that allows input to be passed as an array\n+# to work around shell problems with weird characters in arguments\n+# if the exec returns non-zero we die\n+sub safe_pipe_capture {\n+    my @output;\n+    if (my $pid = open my $child, '-|') {\n+\t@output = (<$child>);\n+\tclose $child or die join(' ',@_).\": $! $?\";\n+    } else {\n+\texec(@_) or die \"$! $?\"; # exec() can fail the executable can't be found\n+    }\n+    return wantarray ? @output : join('',@output);\n+}\n-- \n1.1.6.g170f-dirty\n"}]}