Perl's chop / chomp considered bad, use a regexp instead

Once upon a time, there was chop, which somply chopped off the last
character of $_ or a given variable, and it was used to take off the
EOL character (\n) of strings.

... but then, you had to check for the presence of such character.

So came chomp, the better chop which checks for \n before chopping it
off.  And this worked well, as long as Perl made internally sure that
all EOLs were converted to \n.

These days, though, there seems to be a mixture of perls, so lines
from files in the "wrong" environment might have \r\n as EOL, or just
\r (Mac OS, unless I'm misinformed).

So it's time we went for the more generic variant and use s|\R$||, the
better chomp which recognises all kinds of known EOLs and chops them
off.

A few chops were left alone, as they are use as surgical tools to
remove one last slash or one last comma.

NOTE: \R came with perl 5.10.0.  It means that from now on, our
scripts will fail with any older version.

Reviewed-by: Rich Salz <rsalz@openssl.org>

diff --git a/VMS/VMSify-conf.pl b/VMS/VMSify-conf.pl
index d3be6a29e7f43ff071446d94f3b6286f049d3973..9890362d5bdfd5eec0a4f82b47870f9ced5203c0 100644 (file)
--- a/VMS/VMSify-conf.pl
+++ b/VMS/VMSify-conf.pl
@@ -7,7 +7,7 @@ my @directory_vars = ( "dir", "certs", "crl_dir", "new_certs_dir" );
 my @file_vars = ( "database", "certificate", "serial", "crlnumber",
                  "crl", "private_key", "RANDFILE" );
 while(<STDIN>) {
 my @file_vars = ( "database", "certificate", "serial", "crlnumber",
                  "crl", "private_key", "RANDFILE" );
 while(<STDIN>) {

-    chomp;
+    s|\R$||;

     foreach my $d (@directory_vars) {
        if (/^(\s*\#?\s*${d}\s*=\s*)\.\/([^\s\#]*)([\s\#].*)$/) {
            $_ = "$1sys\\\$disk:\[.$2$3";
     foreach my $d (@directory_vars) {
        if (/^(\s*\#?\s*${d}\s*=\s*)\.\/([^\s\#]*)([\s\#].*)$/) {
            $_ = "$1sys\\\$disk:\[.$2$3";

diff --git a/VMS/translatesyms.pl b/VMS/translatesyms.pl
index 8ffdbd8aa8ca8afa1b567a41345a29ee6948bf99..de3db6ccaf5cee6e56ebd6077c6f8f36bf64dbbf 100644 (file)
--- a/VMS/translatesyms.pl
+++ b/VMS/translatesyms.pl
@@ -28,7 +28,7 @@ my %translations = ();
 open DEMANGLER_DATA, $ARGV[0]
     or die "Couldn't open $ARGV[0]: $!\n";
 while(<DEMANGLER_DATA>) {
 open DEMANGLER_DATA, $ARGV[0]
     or die "Couldn't open $ARGV[0]: $!\n";
 while(<DEMANGLER_DATA>) {

-    chomp;
+    s|\R$||;

     (my $translated, my $original) = split /\$/;
     $translations{$original} = $translated.'$';
 }
     (my $translated, my $original) = split /\$/;
     $translations{$original} = $translated.'$';
 }

diff --git a/apps/CA.pl.in b/apps/CA.pl.in
index 52a97d7345034cb5b0b7ba0f8d3cf6209d9ddb81..fbba457646bb73d030d54d577f79c9296c0ecb6e 100644 (file)
--- a/apps/CA.pl.in
+++ b/apps/CA.pl.in
@@ -121,7 +121,7 @@ if ($WHAT eq '-newcert' ) {
     # ask user for existing CA certificate
     print "CA certificate filename (or enter to create)\n";
     $FILE = <STDIN>;
     # ask user for existing CA certificate
     print "CA certificate filename (or enter to create)\n";
     $FILE = <STDIN>;

-    chop $FILE if $FILE;
+    $FILE = s|\R$|| if $FILE;

     if ($FILE) {
         copy_pemfile($FILE,"${CATOP}/private/$CAKEY", "PRIVATE");
         copy_pemfile($FILE,"${CATOP}/$CACERT", "CERTIFICATE");
     if ($FILE) {
         copy_pemfile($FILE,"${CATOP}/private/$CAKEY", "PRIVATE");
         copy_pemfile($FILE,"${CATOP}/$CACERT", "CERTIFICATE");

diff --git a/crypto/lhash/num.pl b/crypto/lhash/num.pl
index 30fedf9cd5ad8ae6e83fb290d572041bb7129fa9..4440a992dccd4ac84f3531d85b11df2155997af7 100644 (file)
--- a/crypto/lhash/num.pl
+++ b/crypto/lhash/num.pl
@@ -5,7 +5,7 @@
 while (<>)
        {
        next unless /^node/;
 while (<>)
        {
        next unless /^node/;

-       chop;
+       s|\R$||;                # Better chomp

        @a=split;
        $num{$a[3]}++;
        }
        @a=split;
        $num{$a[3]}++;
        }

diff --git a/crypto/objects/obj_dat.pl b/crypto/objects/obj_dat.pl
index d726f2cb76a36be6b37bc920a663bcb9376720d7..0bf1e4878f66dc854df12fb808234395f7efe83b 100644 (file)
--- a/crypto/objects/obj_dat.pl
+++ b/crypto/objects/obj_dat.pl
@@ -257,7 +257,7 @@ foreach (@out)
                                }
                        $out=$t;
                        }
                                }
                        $out=$t;
                        }

-               chop $out;
+               chop $out;      # Get rid of the last comma

                print OUT "$out";
                }
        else
                print OUT "$out";
                }
        else

diff --git a/crypto/objects/objects.pl b/crypto/objects/objects.pl
index ea2caf5ec254af8a4ea0faf7fc5357b0d593ddfe..107647adbc40878fb9d926ff83f03464701f953c 100644 (file)
--- a/crypto/objects/objects.pl
+++ b/crypto/objects/objects.pl
@@ -5,7 +5,7 @@ $max_nid=0;
 $o=0;
 while(<NUMIN>)
        {
 $o=0;
 while(<NUMIN>)
        {

-       chop;
+       s|\R$||;

        $o++;
        s/#.*$//;
        next if /^\s*$/;
        $o++;
        s/#.*$//;
        next if /^\s*$/;


@@ -28,7 +28,7 @@ $Cname="";
 $o=0;
 while (<IN>)
        {
 $o=0;
 while (<IN>)
        {

-       chop;
+       s|\R$||;

        $o++;
         if (/^!module\s+(.*)$/)
                {
        $o++;
         if (/^!module\s+(.*)$/)
                {

diff --git a/crypto/objects/objxref.pl b/crypto/objects/objxref.pl
index 05b987ad16d161261eaaaa6a8c2d360157a424f8..7ebd74cdcc0b3f35fa7d0f8434950bde7dc7c9e4 100644 (file)
--- a/crypto/objects/objxref.pl
+++ b/crypto/objects/objxref.pl
@@ -13,7 +13,7 @@ open(IN, $mac_file) || die "Can't open $mac_file, $!\n";
 
 while (<IN>)
        {
 
 while (<IN>)
        {

-       chomp;
+       s|\R$||;                # Better chomp

        my ($name, $num) = /^(\S+)\s+(\S+)$/;
        $oid_tbl{$name} = $num;
        }
        my ($name, $num) = /^(\S+)\s+(\S+)$/;
        $oid_tbl{$name} = $num;
        }


@@ -25,7 +25,7 @@ my $ln = 1;
 
 while (<IN>)
        {
 
 while (<IN>)
        {

-       chomp;
+       s|\R$||;                # Better chomp

        s/#.*$//;
        next if (/^\S*$/);
        my ($xr, $p1, $p2) = /^(\S+)\s+(\S+)\s+(\S+)/;
        s/#.*$//;
        next if (/^\S*$/);
        my ($xr, $p1, $p2) = /^(\S+)\s+(\S+)\s+(\S+)/;


@@ -112,6 +112,6 @@ sub check_oid
        my ($chk) = @_;
        if (!exists $oid_tbl{$chk})
                {
        my ($chk) = @_;
        if (!exists $oid_tbl{$chk})
                {

-               die "Can't find \"$chk\", $!\n";
+               die "Can't find \"$chk\"\n";

                }
        }
                }
        }

diff --git a/crypto/perlasm/x86_64-xlate.pl b/crypto/perlasm/x86_64-xlate.pl
index 1f5bced8e162e0e35eface009ea414fbdddffbd2..a0b3bc06709dcbfaf6b56d8528749875f1e43584 100755 (executable)
--- a/crypto/perlasm/x86_64-xlate.pl
+++ b/crypto/perlasm/x86_64-xlate.pl
@@ -80,7 +80,7 @@ my $nasm=0;
 
 if    ($flavour eq "mingw64")  { $gas=1; $elf=0; $win64=1;
                                  $prefix=`echo __USER_LABEL_PREFIX__ | $ENV{CC} -E -P -`;
 
 if    ($flavour eq "mingw64")  { $gas=1; $elf=0; $win64=1;
                                  $prefix=`echo __USER_LABEL_PREFIX__ | $ENV{CC} -E -P -`;

-                                 chomp($prefix);
+                                 $prefix =~ s|\R$||; # Better chomp

                                }
 elsif ($flavour eq "macosx")   { $gas=1; $elf=0; $prefix="_"; $decor="L\$"; }
 elsif ($flavour eq "masm")     { $gas=0; $elf=0; $masm=$masmref; $win64=1; $decor="\$L\$"; }
                                }
 elsif ($flavour eq "macosx")   { $gas=1; $elf=0; $prefix="_"; $decor="L\$"; }
 elsif ($flavour eq "masm")     { $gas=0; $elf=0; $masm=$masmref; $win64=1; $decor="\$L\$"; }


@@ -852,7 +852,7 @@ ___
 }
 while($line=<>) {
 
 }
 while($line=<>) {
 

-    chomp($line);
+    $line =~ s|\R$||;           # Better chomp

 
     $line =~ s|[#!].*$||;      # get rid of asm-style comments...
     $line =~ s|/\*.*\*/||;     # ... and C-style comments...
 
     $line =~ s|[#!].*$||;      # get rid of asm-style comments...
     $line =~ s|/\*.*\*/||;     # ... and C-style comments...

diff --git a/util/check-buildinfo.pl b/util/check-buildinfo.pl
index 176b9569006bd07621b415f9b1127a9664e6c55a..f7d3baa9538154ffbaee5f3ad330ae4cb2c775f1 100644 (file)
--- a/util/check-buildinfo.pl
+++ b/util/check-buildinfo.pl
@@ -7,7 +7,7 @@ my $reldir = "";
 my $searchterm = "";
 my $goal = "";
 while (<$minfo>) {
 my $searchterm = "";
 my $goal = "";
 while (<$minfo>) {

-    chomp;
+    s|\R$||;

 
     if (/^RELATIVE_DIRECTORY=(.*)$/) {
         $reldir=$1;
 
     if (/^RELATIVE_DIRECTORY=(.*)$/) {
         $reldir=$1;

diff --git a/util/extract-names.pl b/util/extract-names.pl
index 35bd6ed84326f16ca6ffbdd7e99c8fa1b6a7cf1d..0f69335e96c793706b651c96d08061837dc10979 100644 (file)
--- a/util/extract-names.pl
+++ b/util/extract-names.pl
@@ -2,7 +2,7 @@
 
 $/ = "";                       # Eat a paragraph at once.
 while(<STDIN>) {
 
 $/ = "";                       # Eat a paragraph at once.
 while(<STDIN>) {

-    chop;
+    s|\R$||;

     s/\n/ /gm;
     if (/^=head1 /) {
        $name = 0;
     s/\n/ /gm;
     if (/^=head1 /) {
        $name = 0;

diff --git a/util/files.pl b/util/files.pl
index d5c78bafc1bfecc696c3cc594fc45937ef51b083..d984196616053efd2d73e23f4b07ac049ea41f66 100755 (executable)
--- a/util/files.pl
+++ b/util/files.pl
@@ -13,7 +13,7 @@ while ($ARGV[0] =~ /^([^\s=]+)\s*=\s*(.*)$/)
 $s="";
 while (<>)
        {
 $s="";
 while (<>)
        {

-       chop;
+       s|\R$||;

        s/#.*//;
        if (/^([^\s=]+)\s*=\s*(.*)$/)
                {
        s/#.*//;
        if (/^([^\s=]+)\s*=\s*(.*)$/)
                {


@@ -23,10 +23,10 @@ while (<>)
                        {
                        if ($b =~ /\\$/)
                                {
                        {
                        if ($b =~ /\\$/)
                                {

-                               chop($b);
+                               $b=$`; # Keep what is before the backslash

                                $o.=$b." ";
                                $b=<>;
                                $o.=$b." ";
                                $b=<>;

-                               chop($b);
+                               $b =~ s|\R$||; # Better chomp

                                }
                        else
                                {
                                }
                        else
                                {


@@ -43,7 +43,7 @@ while (<>)
                }
        }
 
                }
        }
 

-$pwd=`pwd`; chop($pwd);
+$pwd=`pwd`; $pwd =~ s|\R$||;

 
 if ($sym{'TOP'} eq ".")
        {
 
 if ($sym{'TOP'} eq ".")
        {


@@ -55,7 +55,7 @@ else  {
        @_=split(/\//,$pwd);
        $z=$#_-$n+1;
        foreach $i ($z .. $#_) { $dir.=$_[$i]."/"; }
        @_=split(/\//,$pwd);
        $z=$#_-$n+1;
        foreach $i ($z .. $#_) { $dir.=$_[$i]."/"; }

-       chop($dir);
+       chop($dir);             # Remove the last slash

        }
 
 print "RELATIVE_DIRECTORY=$dir\n";
        }
 
 print "RELATIVE_DIRECTORY=$dir\n";

diff --git a/util/fipslink.pl b/util/fipslink.pl
index 4a88fc6d77e07d36b25d95fdb7c8f782e7666d79..7b16e04fb9da7a752254600f6b7d096e2d653780 100644 (file)
--- a/util/fipslink.pl
+++ b/util/fipslink.pl
@@ -59,7 +59,7 @@ open my $sha1_res, '<', $fips_target.".sha1" or die "Get hash failure";
 $fips_hash=<$sha1_res>;
 close $sha1_res;
 unlink $fips_target.".sha1";
 $fips_hash=<$sha1_res>;
 close $sha1_res;
 unlink $fips_target.".sha1";

-chomp $fips_hash;
+$fips_hash =~ s|\R$||;          # Better chomp

 die "Get hash failure" if $? != 0;
 
 
 die "Get hash failure" if $? != 0;
 
 


@@ -97,8 +97,8 @@ sub check_hash
        $hashfile = <IN>;
        close IN;
        $hashval = `$sha1_exe ${fips_libdir}/$filename`;
        $hashfile = <IN>;
        close IN;
        $hashval = `$sha1_exe ${fips_libdir}/$filename`;

-       chomp $hashfile;
-       chomp $hashval;
+       $hashfile =~ s|\R$||;    # Better chomp
+       $hashval =~ s|\R$||;     # Better chomp

        $hashfile =~ s/^.*=\s+//;
        $hashval =~ s/^.*=\s+//;
        die "Invalid hash syntax in file" if (length($hashfile) != 40);
        $hashfile =~ s/^.*=\s+//;
        $hashval =~ s/^.*=\s+//;
        die "Invalid hash syntax in file" if (length($hashfile) != 40);

diff --git a/util/mk1mf.pl b/util/mk1mf.pl
index 41441305677e8add95f4fa002c333f62e9ede3f6..3a9f0d76b8267f3f82ea0e570ef18943deef8890 100755 (executable)
--- a/util/mk1mf.pl
+++ b/util/mk1mf.pl
@@ -553,7 +553,7 @@ if ($fips)
                        {
                        open (IN, "util/fipslib_path.txt") || fipslib_error();
                        $fipslibdir = <IN>;
                        {
                        open (IN, "util/fipslib_path.txt") || fipslib_error();
                        $fipslibdir = <IN>;

-                       chomp $fipslibdir;
+                       $fipslibdir =~ s|\R$||;

                        close IN;
                        }
                fips_check_files($fipslibdir,
                        close IN;
                        }
                fips_check_files($fipslibdir,


@@ -1159,7 +1159,7 @@ sub do_defs
                elsif ($var eq "SSLOBJ")
                        { $ret.="\$(OBJ_D)\\\$(SSL).res "; }
                }
                elsif ($var eq "SSLOBJ")
                        { $ret.="\$(OBJ_D)\\\$(SSL).res "; }
                }

-       chomp($ret);
+       chomp($ret);            # Does this actually do something? /RL

        $ret.="\n\n";
        return($ret);
        }
        $ret.="\n\n";
        return($ret);
        }

diff --git a/util/mkdef.pl b/util/mkdef.pl
index aa85ec8251f874df225281414f5934f80878f158..b5ebc18b8e2edab54c9e49bb45ccec91b3181091 100755 (executable)
--- a/util/mkdef.pl
+++ b/util/mkdef.pl
@@ -459,7 +459,7 @@ sub do_defs
                        if($parens > 0) {
                                #Inside a DEPRECATEDIN
                                $stored_multiline .= $_;
                        if($parens > 0) {
                                #Inside a DEPRECATEDIN
                                $stored_multiline .= $_;

-                               chomp $stored_multiline;
+                               $stored_multiline =~ s|\R$||; # Better chomp

                                print STDERR "DEBUG: Continuing multiline DEPRECATEDIN: $stored_multiline\n" if $debug;
                                $parens = count_parens($stored_multiline);
                                if ($parens == 0) {
                                print STDERR "DEBUG: Continuing multiline DEPRECATEDIN: $stored_multiline\n" if $debug;
                                $parens = count_parens($stored_multiline);
                                if ($parens == 0) {


@@ -480,9 +480,7 @@ sub do_defs
                        }
 
                        if (/\\$/) {
                        }
 
                        if (/\\$/) {

-                               chomp; # remove eol
-                               chop; # remove ending backslash
-                               $line = $_;
+                               $line = $`; # keep what was before the backslash

                                next;
                        }
 
                                next;
                        }
 


@@ -499,7 +497,7 @@ sub do_defs
                                $cpp++ if /^#\s*if/;
                                $cpp-- if /^#\s*endif/;
                                next;
                                $cpp++ if /^#\s*if/;
                                $cpp-- if /^#\s*endif/;
                                next;

-                       }
+                       }

                        $cpp = 1 if /^#.*ifdef.*cplusplus/;
 
                        s/{[^{}]*}//gs;                      # ignore {} blocks
                        $cpp = 1 if /^#.*ifdef.*cplusplus/;
 
                        s/{[^{}]*}//gs;                      # ignore {} blocks


@@ -867,7 +865,7 @@ sub do_defs
                                                        \@current_algorithms);
                                        } else {
                                                $stored_multiline = $_;
                                                        \@current_algorithms);
                                        } else {
                                                $stored_multiline = $_;

-                                               chomp $stored_multiline;
+                                               $stored_multiline =~ s|\R$||;

                                                print STDERR "DEBUG: Found multiline DEPRECATEDIN starting with: $stored_multiline\n" if $debug;
                                                next;
                                        }
                                                print STDERR "DEBUG: Found multiline DEPRECATEDIN starting with: $stored_multiline\n" if $debug;
                                                next;
                                        }


@@ -1365,7 +1363,7 @@ sub load_numbers
 
        open(IN,"<$name") || die "unable to open $name:$!\n";
        while (<IN>) {
 
        open(IN,"<$name") || die "unable to open $name:$!\n";
        while (<IN>) {

-               chop;
+               s|\R$||;        # Better chomp

                s/#.*$//;
                next if /^\s*$/;
                @a=split;
                s/#.*$//;
                next if /^\s*$/;
                @a=split;

diff --git a/util/mkerr.pl b/util/mkerr.pl
index 13c9974bcea80779ebf386eca7e60411385bdeb5..939a87c07cf2afef79ef6f223e01449fdfc08063 100644 (file)
--- a/util/mkerr.pl
+++ b/util/mkerr.pl
@@ -556,7 +556,7 @@ EOF
        if (open(IN,"<$cfile")) {
                my $line = "";
                while (<IN>) {
        if (open(IN,"<$cfile")) {
                my $line = "";
                while (<IN>) {

-                       chomp;
+                       s|\R$||; # Better chomp

                        $_ = $line . $_;
                        $line = "";
                        if (/{ERR_(FUNC|REASON)\(/) {
                        $_ = $line . $_;
                        $line = "";
                        if (/{ERR_(FUNC|REASON)\(/) {

diff --git a/util/mkfiles.pl b/util/mkfiles.pl
index d668316d697de59ca5cf6de8c093700edfb6693c..4fbe29ac755aa84e210209507020ec1ed252721e 100755 (executable)
--- a/util/mkfiles.pl
+++ b/util/mkfiles.pl
@@ -95,7 +95,7 @@ my $s="";
 
 while (<IN>)
        {
 
 while (<IN>)
        {

-       chop;
+       s|\R$||;

        s/#.*//;
        if (/^([^\s=]+)\s*=\s*(.*)$/)
                {
        s/#.*//;
        if (/^([^\s=]+)\s*=\s*(.*)$/)
                {


@@ -105,10 +105,10 @@ while (<IN>)
                        {
                        if ($b =~ /\\$/)
                                {
                        {
                        if ($b =~ /\\$/)
                                {

-                               chop($b);
+                               $b=$`;

                                $o.=$b." ";
                                $b=<IN>;
                                $o.=$b." ";
                                $b=<IN>;

-                               chop($b);
+                               $b =~ s|\R$||;

                                }
                        else
                                {
                                }
                        else
                                {

diff --git a/util/selftest.pl b/util/selftest.pl
index 59842efae8f041f06c1637f269e9e0967db8395b..06d494a2fb93e13c54fc46e242ccced85eb485be 100644 (file)
--- a/util/selftest.pl
+++ b/util/selftest.pl
@@ -54,7 +54,7 @@ $cversion=`$cc -V |head -1` if $cversion =~ "Error";
 $cversion=`$cc --version` if $cversion eq "";
 $cversion =~ s/Reading specs.*\n//;
 $cversion =~ s/usage.*\n//;
 $cversion=`$cc --version` if $cversion eq "";
 $cversion =~ s/Reading specs.*\n//;
 $cversion =~ s/usage.*\n//;

-chomp $cversion;
+$cversion =~ s|\R$||;

 
 if (open(IN,"<CHANGES")) {
     while(<IN>) {
 
 if (open(IN,"<CHANGES")) {
     while(<IN>) {

diff --git a/util/sp-diff.pl b/util/sp-diff.pl
index 9d6c60387fa1c862f4c3fe4a4fe86a30c48e60ee..57e635bca38fef59c2b99572fdfb022e6cf44131 100755 (executable)
--- a/util/sp-diff.pl
+++ b/util/sp-diff.pl
@@ -54,7 +54,7 @@ sub loadfile
                $header=0 if /^[dr]sa/;
                if (/^type/) { $header=0; next; }
                next if $header;
                $header=0 if /^[dr]sa/;
                if (/^type/) { $header=0; next; }
                next if $header;

-               chop;
+               s|\R$||;

                @a=split;
                if ($a[0] =~ /^[dr]sa$/)
                        {
                @a=split;
                if ($a[0] =~ /^[dr]sa$/)
                        {