Re: [PATCH] Convert open("-|") to qx{} calls

On 2/23/06, Johannes Schindelin [off-list ref] wrote:

      Since of these 4, I only use cvsimport myself, I could only test
      that. Could someone who uses the others give them a hard beating?

I can't really test them (no svn and cvs, and locked down network), but I took
a look at the patches. Hope it helps.

git-cvsimport:

-               open(F,"git-cat-file commit $ftag |");
-               while(<F>) {
+               foreach (qx{git-cat-file commit $ftag}) {
                        next unless /^author\s.*\s(\d+)\s[-+]\d{4}$/;

Are you sure you don't need quoting/safe pipe here?
Or is it a CVS tag?

+} else {
+    @input = qx{cvsps --norc opt -u -A --root $opt_d $cvs_tree};
+    !$? or exit $?;

Same here. $cvs_tree can contain any filesystem-allowed character.

git-svnimport:

-                       my $sha = <$F>;
+                       my $sha = qx{git-hash-object -w $tmpname};
+                       !$? or exit $?;

Is $tmpname safe?

-       my $sha = <$F>;
+       my $sha = qx{git-hash-object -w $name};
+       !$? or exit $?;

Is $name safe?

-       while(<$f>) {
+       foreach (qx{git-ls-tree -r -z $gitrev $srcpath}) {
                chomp;

Is $srcpath safe?

-                       while(<$F>) {
+                       foreach (qx{git-ls-files -z @o1}) {

@o1 must contain filenames. Can be dangerous