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 <[email protected]>
kcfigaro · Feb 11, 2016 · 9ba96fb · 9ba96fb
1 parent c15e95a
commit 9ba96fb
Show file tree

Hide file tree

Showing 18 changed files with 35 additions and 37 deletions.
diff --git a/VMS/VMSify-conf.pl b/VMS/VMSify-conf.pl
@@ -7,7 +7,7 @@
 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";

diff --git a/VMS/translatesyms.pl b/VMS/translatesyms.pl
@@ -28,7 +28,7 @@
 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.'$';
 }

diff --git 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>;
-    chop $FILE if $FILE;
+    $FILE = s|\R$|| if $FILE;
     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
@@ -5,7 +5,7 @@
 while (<>)
 	{
 	next unless /^node/;
-	chop;
+	s|\R$||;                # Better chomp
 	@a=split;
 	$num{$a[3]}++;
 	}

diff --git a/crypto/objects/obj_dat.pl b/crypto/objects/obj_dat.pl
@@ -257,7 +257,7 @@ sub expand_obj
 				}
 			$out=$t;
 			}
-		chop $out;
+		chop $out;      # Get rid of the last comma
 		print OUT "$out";
 		}
 	else

diff --git a/crypto/objects/objects.pl b/crypto/objects/objects.pl
@@ -5,7 +5,7 @@
 $o=0;
 while(<NUMIN>)
 	{
-	chop;
+	s|\R$||;
 	$o++;
 	s/#.*$//;
 	next if /^\s*$/;
@@ -28,7 +28,7 @@
 $o=0;
 while (<IN>)
 	{
-	chop;
+	s|\R$||;
 	$o++;
         if (/^!module\s+(.*)$/)
 		{

diff --git a/crypto/objects/objxref.pl b/crypto/objects/objxref.pl
@@ -13,7 +13,7 @@
 
 while (<IN>)
 	{
-	chomp;
+	s|\R$||;                # Better chomp
 	my ($name, $num) = /^(\S+)\s+(\S+)$/;
 	$oid_tbl{$name} = $num;
 	}
@@ -25,7 +25,7 @@
 
 while (<IN>)
 	{
-	chomp;
+	s|\R$||;                # Better chomp
 	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})
 		{
-		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
@@ -80,7 +80,7 @@
 
 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\$"; }
@@ -852,7 +852,7 @@ sub rxb {
 }
 while($line=<>) {
 
-    chomp($line);
+    $line =~ s|\R$||;           # Better chomp
 
     $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
@@ -7,7 +7,7 @@
 my $searchterm = "";
 my $goal = "";
 while (<$minfo>) {
-    chomp;
+    s|\R$||;
 
     if (/^RELATIVE_DIRECTORY=(.*)$/) {
         $reldir=$1;

diff --git a/util/extract-names.pl b/util/extract-names.pl
@@ -2,7 +2,7 @@
 
 $/ = "";			# Eat a paragraph at once.
 while(<STDIN>) {
-    chop;
+    s|\R$||;
     s/\n/ /gm;
     if (/^=head1 /) {
 	$name = 0;

diff --git a/util/files.pl b/util/files.pl
@@ -13,7 +13,7 @@
 $s="";
 while (<>)
 	{
-	chop;
+	s|\R$||;
 	s/#.*//;
 	if (/^([^\s=]+)\s*=\s*(.*)$/)
 		{
@@ -23,10 +23,10 @@
 			{
 			if ($b =~ /\\$/)
 				{
-				chop($b);
+				$b=$`; # Keep what is before the backslash
 				$o.=$b." ";
 				$b=<>;
-				chop($b);
+				$b =~ s|\R$||; # Better chomp
 				}
 			else
 				{
@@ -43,7 +43,7 @@
 		}
 	}
 
-$pwd=`pwd`; chop($pwd);
+$pwd=`pwd`; $pwd =~ s|\R$||;
 
 if ($sym{'TOP'} eq ".")
 	{
@@ -55,7 +55,7 @@
 	@_=split(/\//,$pwd);
 	$z=$#_-$n+1;
 	foreach $i ($z .. $#_) { $dir.=$_[$i]."/"; }
-	chop($dir);
+	chop($dir);             # Remove the last slash
 	}
 
 print "RELATIVE_DIRECTORY=$dir\n";

diff --git a/util/fipslink.pl b/util/fipslink.pl
@@ -59,7 +59,7 @@ sub check_env
 $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;
 
 
@@ -97,8 +97,8 @@ sub check_hash
 	$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);

diff --git a/util/mk1mf.pl b/util/mk1mf.pl
@@ -553,7 +553,7 @@
 			{
 			open (IN, "util/fipslib_path.txt") || fipslib_error();
 			$fipslibdir = <IN>;
-			chomp $fipslibdir;
+			$fipslibdir =~ s|\R$||;
 			close IN;
 			}
 		fips_check_files($fipslibdir,
@@ -1159,7 +1159,7 @@ sub do_defs
 		elsif ($var eq "SSLOBJ")
 			{ $ret.="\$(OBJ_D)\\\$(SSL).res "; }
 		}
-	chomp($ret);
+	chomp($ret);            # Does this actually do something? /RL
 	$ret.="\n\n";
 	return($ret);
 	}

diff --git a/util/mkdef.pl b/util/mkdef.pl
@@ -459,7 +459,7 @@ sub do_defs
 			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) {
@@ -480,9 +480,7 @@ sub do_defs
 			}
 
 			if (/\\$/) {
-				chomp; # remove eol
-				chop; # remove ending backslash
-				$line = $_;
+				$line = $`; # keep what was before the backslash
 				next;
 			}
 
@@ -499,7 +497,7 @@ sub do_defs
 				$cpp++ if /^#\s*if/;
 				$cpp-- if /^#\s*endif/;
 				next;
-	    		}
+			}
 			$cpp = 1 if /^#.*ifdef.*cplusplus/;
 
 			s/{[^{}]*}//gs;                      # ignore {} blocks
@@ -867,7 +865,7 @@ sub do_defs
 							\@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;
 					}
@@ -1365,7 +1363,7 @@ sub load_numbers
 
 	open(IN,"<$name") || die "unable to open $name:$!\n";
 	while (<IN>) {
-		chop;
+		s|\R$||;        # Better chomp
 		s/#.*$//;
 		next if /^\s*$/;
 		@a=split;

diff --git a/util/mkerr.pl b/util/mkerr.pl
@@ -556,7 +556,7 @@
 	if (open(IN,"<$cfile")) {
 		my $line = "";
 		while (<IN>) {
-			chomp;
+			s|\R$||; # Better chomp
 			$_ = $line . $_;
 			$line = "";
 			if (/{ERR_(FUNC|REASON)\(/) {

diff --git a/util/mkfiles.pl b/util/mkfiles.pl
@@ -95,7 +95,7 @@ sub files_dir
 
 while (<IN>)
 	{
-	chop;
+	s|\R$||;
 	s/#.*//;
 	if (/^([^\s=]+)\s*=\s*(.*)$/)
 		{
@@ -105,10 +105,10 @@ sub files_dir
 			{
 			if ($b =~ /\\$/)
 				{
-				chop($b);
+				$b=$`;
 				$o.=$b." ";
 				$b=<IN>;
-				chop($b);
+				$b =~ s|\R$||;
 				}
 			else
 				{

diff --git a/util/selftest.pl b/util/selftest.pl
@@ -54,7 +54,7 @@
 $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>) {

diff --git 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;
-		chop;
+		s|\R$||;
 		@a=split;
 		if ($a[0] =~ /^[dr]sa$/)
 			{