Review and fix regular expressions

Francois Gouget fgouget at free.fr
Fri Oct 22 11:06:37 CDT 2004


Working on winapi_check I found the following:

    if($called_name =~ /^if|for|while|switch|sizeof$/) {

This regular expression matches any string
  * starting with 'if'
  * containing 'for', 'while' or 'switch' anywhere in the string
  * or ending with 'sizeof'

This is clearly not what was meant.

So I did a review of our regular expressions and found a lot of similar 
bugs and fixed them. I also replaced regular expression matces with 
simple comparisons where the use of a regular expression is unnecessary.


Changelog:

  * tools/winapi/c_parser.pm
    tools/winapi/make_parser.pm
    tools/winapi/msvcmaker
    tools/winapi/winapi.pm
    tools/winapi/winapi_extract
    tools/winapi_check/modules.pm
    tools/winapi_check/nativeapi.pm
    tools/winapi_check/winapi_check
    tools/winapi_check/winapi_function.pm
    tools/winapi_check/winapi_local.pm

    Review and fix regular expressions of the form /^foo|bar$/.
    Replace regular expressions with simple string comparisons where 
possible.
    Use '(?:subregexp)' instead of '(subregexp)' wherever possible.
   'dlls/gdi' does not have a win16drv subdirectory anymore so simplify 
regular expressions accordingly.


-- 
Francois Gouget         fgouget at free.fr        http://fgouget.free.fr/
                           La terre est une b\xEAta...
-------------- next part --------------
Index: tools/winapi/c_parser.pm
===================================================================
RCS file: /var/cvs/wine/tools/winapi/c_parser.pm,v
retrieving revision 1.15
diff -u -r1.15 c_parser.pm
--- tools/winapi/c_parser.pm	8 Sep 2004 01:27:24 -0000	1.15
+++ tools/winapi/c_parser.pm	22 Oct 2004 08:36:17 -0000
@@ -1199,7 +1199,7 @@
 			  'long(?=\s+double\b|\s+int\b|\s+long\b))(?=\b)',
 			  \$_, \$line, \$column, \$match))
     {
-	if($match =~ /^extern|static$/) {
+	if($match =~ /^(?:extern|static)$/) {
 	    if(!$linkage) {
 		$linkage = $match;
 	    }
@@ -1956,13 +1956,13 @@
 			  'long(?=\s+double\b|\s+int\b|\s+long\b))(?=\b)',
 			  \$_, \$line, \$column, \$match))
     {
-	if ($match =~ /^extern|static$/) {
+	if ($match =~ /^(?:extern|static)$/) {
 	    if (!$linkage) {
 		$linkage = $match;
 	    } else {
 		$self->_parse_c_warning($_, $line, $column, "repeated linkage (ignored): $match");
 	    }
-	} elsif ($match =~ /^signed|unsigned$/) {
+	} elsif ($match =~ /^(?:signed|unsigned)$/) {
 	    if (!$sign) {
 		$sign = "$match ";
 	    } else {
Index: tools/winapi/make_parser.pm
===================================================================
RCS file: /var/cvs/wine/tools/winapi/make_parser.pm,v
retrieving revision 1.9
diff -u -r1.9 make_parser.pm
--- tools/winapi/make_parser.pm	1 Jun 2002 02:55:53 -0000	1.9
+++ tools/winapi/make_parser.pm	22 Oct 2004 08:38:36 -0000
@@ -103,15 +101,15 @@
 	    $progress .= "$tool: ";
 	}
 
-	if($tool =~ /^cd|make$/) {
+	if($tool =~ /^(?:cd|make)$/) {
 	    # Nothing
-	} elsif($tool =~ /^ld$/) {
+	} elsif($tool eq "ld"/) {
 	    foreach my $file (@{$read_files}) {
 		$output->lazy_progress("${progress}reading '$file'");
 	    }
 	    my $file = $$write_files[0];
 	    $output->progress("$progress: writing '$file'");
-	} elsif($tool =~ /^rm$/) {
+	} elsif($tool eq "rm") {
 	    foreach my $file (@{$remove_files}) {
 		$output->lazy_progress("${progress}removing '$file'");
 	    }
@@ -160,13 +158,13 @@
 	# Nothing
     } elsif($tool eq "gcc" && /^(?:In file included |\s*)from (.+?):(\d+)[,:]$/) {
 	# Nothing
-    } elsif($tool =~ /^gcc|ld$/ && s/^(.+?\.s?o)(?:\(.*?\))?:\s*//) {
+    } elsif($tool =~ /^(?:gcc|ld)$/ && s/^(.+?\.s?o)(?:\(.*?\))?:\s*//) {
 	$tool = "ld";
 	ld_output($1, $_)
-    } elsif($tool =~ /^gcc|ld$/ && s/^(.*?)ld:\s*//) {
+    } elsif($tool =~ /^(?:gcc|ld)$/ && s/^(.*?)ld:\s*//) {
 	$tool = "ld";
 	ld_output("", $_)
-    } elsif($tool =~ /^gcc|ld$/ && s/^collect2:\s*//) {
+    } elsif($tool =~ /^(?:gcc|ld)$/ && s/^collect2:\s*//) {
 	$tool = "ld";
 	ld_output("collect2", $_);
     } elsif($tool eq "gcc" && s/^(.+?\.[chly]):\s*//) {
Index: tools/winapi/msvcmaker
===================================================================
RCS file: /var/cvs/wine/tools/winapi/msvcmaker,v
retrieving revision 1.33
diff -u -r1.33 msvcmaker
--- tools/winapi/msvcmaker	11 Oct 2004 19:51:43 -0000	1.33
+++ tools/winapi/msvcmaker	22 Oct 2004 09:42:17 -0000
@@ -57,7 +57,7 @@
 	/^$/ && next;         # skip empty lines
 
 	if($header)  {
-	    if(/^\d+|@/) {
+	    if(/^(?:\d+|@)/) {
 		$header = 0;
 		$lookahead = 1;
 	    }
@@ -478,7 +478,7 @@
     my @header_files = @{$modules{$module}{header_files}};
     my @resource_files = @{$modules{$module}{resource_files}};
 
-    if ($project !~ /^(?:wine(?:_unicode)?|wine(?:build|runtests|test))$/ &&
+    if ($project !~ /^wine(?:_unicode|build|runtests|test)?$/ &&
         $project !~ /^(?:gdi32|ntdll|user32)_.+?$/ &&
         $project !~ /_test$/)
     {
@@ -489,7 +489,7 @@
 
     my $no_cpp = 1;
     my $no_msvc_headers = 1;
-    if ($project =~ /^(?:wine(?:runtests|test))$/ || $project =~ /_test$/) {
+    if ($project =~ /^wine(?:runtests|test)$/ || $project =~ /_test$/) {
 	$no_msvc_headers = 0;
     }
 
@@ -684,23 +684,23 @@
 	    push @defines2, "__WINETEST_OUTPUT_DIR=\\\"$output_dir\\\"";
 	    push @defines2, qw(__i386__ _X86_);
 
-	    if($project =~ /^gdi32_(?:enhmfdrv|mfdrv|win16drv)$/) {
+	    if($project =~ /^gdi32_(?:enhmfdrv|mfdrv)$/) {
 		push @includes, "..";
 	    }
 
-	    if($project =~ /^user32_(?:windows)$/) {
+	    if($project eq "user32_windows") {
 		push @includes, "..\\dlls\\user";
 	    }
 
-	    if($project =~ /^user32_dde$/) {
+	    if($project eq "user32_dde") {
 		push @includes, "..";
 	    }
 
 	    if ($project =~ /_test$/) {
 		push @includes, "$msvc_wine_dir\\$output_dir";
-	    } 
+	    }
 
-	    if (!$msvc_headers || $project =~ /^winetest$/) {
+	    if (!$msvc_headers || $project eq "winetest") {
 		push @includes, $wine_include_dir;
 	    }
 	}
Index: tools/winapi/winapi.pm
===================================================================
RCS file: /var/cvs/wine/tools/winapi/winapi.pm,v
retrieving revision 1.11
diff -u -r1.11 winapi.pm
--- tools/winapi/winapi.pm	29 Sep 2003 20:15:24 -0000	1.11
+++ tools/winapi/winapi.pm	22 Oct 2004 09:33:51 -0000
@@ -442,7 +442,7 @@
             } else {
                 $$module_external_calling_convention{$module}{"\@$ordinal"} = "extern";
             }
-	} elsif(/^(\d+|@)\s+(equate|variable)/) {
+	} elsif(/^(?:\d+|@)\s+(?:equate|variable)/) {
 	    # ignore
 	} else {
 	    my $next_line = <IN>;
Index: tools/winapi/winapi_extract
===================================================================
RCS file: /var/cvs/wine/tools/winapi/winapi_extract,v
retrieving revision 1.21
diff -u -r1.21 winapi_extract
--- tools/winapi/winapi_extract	15 Oct 2002 02:15:35 -0000	1.21
+++ tools/winapi/winapi_extract	22 Oct 2004 09:39:23 -0000
@@ -75,14 +75,14 @@
 	    /^$/ && next;         # skip empty lines
 
 	    if($header)  {
-	        if(/^\d+|@/) {
+	        if(/^(?:\d+|@)/) {
 		    $header = 0;
 		    $lookahead = 1;
 		}
 		next;
 	    }
 
-	    if(/^(@|\d+)\s+stdcall\s+(\w+)\s*\(\s*([^\)]*)\s*\)/) {
+	    if(/^(\d+|@)\s+stdcall\s+(\w+)\s*\(\s*([^\)]*)\s*\)/) {
 		my $ordinal = $1;
 		my $name = $2;
 		my @args = split(/\s+/, $3);
@@ -559,7 +559,7 @@
              foreach my $external_name ($winapi->all_functions_in_module($module)) {
 		 my $external_calling_convention =
 		     $winapi->function_external_calling_convention_in_module($module, $external_name);
-		 if($external_calling_convention !~ /^forward|stub$/) {
+		 if($external_calling_convention !~ /^(?:forward|stub)$/) {
 		     if($module_pseudo_stub{$module}{$external_name}) {
 			 $external_calling_convention = "pseudo_stub";
 		     }
Index: tools/winapi_check/modules.pm
===================================================================
RCS file: /var/cvs/wine/tools/winapi_check/modules.pm,v
retrieving revision 1.22
diff -u -r1.22 modules.pm
--- tools/winapi_check/modules.pm	7 Oct 2004 18:53:56 -0000	1.22
+++ tools/winapi_check/modules.pm	22 Oct 2004 09:45:13 -0000
@@ -71,7 +71,7 @@
 	/^$/ && next;
 
 	if($header)  {
-	    if(/^\d+|@/) { $header = 0; $lookahead = 1; }
+	    if(/^(?:\d+|@)/) { $header = 0; $lookahead = 1; }
 	    next;
 	}
 
Index: tools/winapi_check/nativeapi.pm
===================================================================
RCS file: /var/cvs/wine/tools/winapi_check/nativeapi.pm,v
retrieving revision 1.17
diff -u -r1.17 nativeapi.pm
--- tools/winapi_check/nativeapi.pm	7 Oct 2004 18:53:56 -0000	1.17
+++ tools/winapi_check/nativeapi.pm	22 Oct 2004 08:25:52 -0000
@@ -207,7 +207,7 @@
 
     my @messages;
     foreach my $name (sort(keys(%$conditionals))) {
-	if($name =~ /^const|inline|size_t$/) { next; }
+	if($name =~ /^(?:const|inline|size_t)$/) { next; }
 
 	if(0 && !$$conditional_found{$name}) {
 	    push @messages, "config.h.in: conditional $name not used\n";
Index: tools/winapi_check/winapi_check
===================================================================
RCS file: /var/cvs/wine/tools/winapi_check/winapi_check,v
retrieving revision 1.67
diff -u -r1.67 winapi_check
--- tools/winapi_check/winapi_check	21 Oct 2004 20:57:53 -0000	1.67
+++ tools/winapi_check/winapi_check	22 Oct 2004 08:29:37 -0000
@@ -477,7 +477,7 @@
 
 	if($options->config) {
 	    if(!$nativeapi->is_conditional($_)) {
-		if(/^HAVE_/ && !/^HAVE_(IPX|MESAGL|BUGGY_MESAGL|WINE_CONSTRUCTOR)$/)
+		if(/^HAVE_/ && !/^HAVE_(?:IPX|MESAGL|BUGGY_MESAGL|WINE_CONSTRUCTOR)$/)
 		{
 		    $output->write("$file: $_ is not declared as a conditional\n");
 		}
@@ -556,7 +556,7 @@
 
 		if($check_protection && $header) {
 		    if((-e "$wine_dir/include/$header" || -e "$wine_dir/$file_dir/$header")) {
-			if($header !~ /^(oleauto\.h|win(?:base|def|error|gdi|nls|nt|user)\.h)$/ &&
+			if($header !~ /^(?:oleauto\.h|win(?:base|def|error|gdi|nls|nt|user)\.h)$/ &&
 			   $file_dir !~ /tests$/)
 			{
 			    $output->write("$file: #include \<$header\> is a local include\n");
Index: tools/winapi_check/winapi_function.pm
===================================================================
RCS file: /var/cvs/wine/tools/winapi_check/winapi_function.pm,v
retrieving revision 1.17
diff -u -r1.17 winapi_function.pm
--- tools/winapi_check/winapi_function.pm	7 Oct 2004 18:53:56 -0000	1.17
+++ tools/winapi_check/winapi_function.pm	22 Oct 2004 08:32:49 -0000
@@ -304,15 +304,15 @@
     }
 
     local $_ = $self->calling_convention;
-    if(/^__cdecl$/) {
+    if($_ eq "__cdecl") {
 	return "cdecl";
-    } elsif(/^VFWAPIV|WINAPIV$/) {
+    } elsif(/^(?:VFWAPIV|WINAPIV)$/) {
 	if(!defined($suffix)) { return undef; }
 	return "pascal$suffix"; # FIXME: Is this correct?
-    } elsif(/^__stdcall|VFWAPI|WINAPI|CALLBACK$/) {
+    } elsif(/^(?:__stdcall|VFWAPI|WINAPI|CALLBACK)$/) {
 	if(!defined($suffix)) { return undef; }
 	return "pascal$suffix";
-    } elsif(/^__asm$/) {
+    } elsif($_ eq "__asm") {
 	return "asm";
     } else {
 	return "cdecl";
@@ -323,13 +323,13 @@
     my $self = shift;
 
     local $_ = $self->calling_convention;
-    if(/^__cdecl$/) {
+    if($_ eq "__cdecl") {
 	return "cdecl";
-    } elsif(/^VFWAPIV|WINAPIV$/) {
+    } elsif(/^(?:VFWAPIV|WINAPIV)$/) {
 	return "varargs";
-    } elsif(/^__stdcall|VFWAPI|WINAPI|CALLBACK$/) {
+    } elsif(/^(?:__stdcall|VFWAPI|WINAPI|CALLBACK)$/) {
 	return "stdcall";
-    } elsif(/^__asm$/) {
+    } elsif($_ eq "__asm") {
 	return "asm";
     } else {
 	return "cdecl";
Index: tools/winapi_check/winapi_local.pm
===================================================================
RCS file: /var/cvs/wine/tools/winapi_check/winapi_local.pm,v
retrieving revision 1.41
diff -u -r1.41 winapi_local.pm
--- tools/winapi_check/winapi_local.pm	21 Oct 2004 20:58:13 -0000	1.41
+++ tools/winapi_check/winapi_local.pm	22 Oct 2004 09:47:45 -0000
@@ -108,39 +108,39 @@
     }
 
     my $segmented = 0;
-    if(defined($implemented_return_kind) && $implemented_return_kind =~ /^segptr|segstr$/) {
+    if(defined($implemented_return_kind) && $implemented_return_kind =~ /^seg[sp]tr$/) {
 	$segmented = 1;
     }
 
     my $implemented_calling_convention;
     if($winapi->name eq "win16") {
-	if($calling_convention =~ /^__cdecl$/) {
+	if($calling_convention eq "__cdecl") {
 	    $implemented_calling_convention = "cdecl";
-	} elsif($calling_convention =~ /^VFWAPIV|WINAPIV$/) {
+	} elsif($calling_convention =~ /^(?:VFWAPIV|WINAPIV)$/) {
 	    $implemented_calling_convention = "varargs";
-	} elsif($calling_convention =~ /^__stdcall|VFWAPI|WINAPI|CALLBACK$/) {
-	    if(defined($implemented_return_kind) && $implemented_return_kind =~ /^s_word|word|void$/) {
+	} elsif($calling_convention =~ /^(?:__stdcall|VFWAPI|WINAPI|CALLBACK)$/) {
+	    if(defined($implemented_return_kind) && $implemented_return_kind =~ /^(?:s_word|word|void)$/) {
 		$implemented_calling_convention = "pascal16";
 	    } else {
 		$implemented_calling_convention = "pascal";
 	    }
-	} elsif($calling_convention =~ /^__asm$/) {
+	} elsif($calling_convention eq "__asm") {
     	    $implemented_calling_convention = "asm";
 	} else {
     	    $implemented_calling_convention = "cdecl";
 	}
     } elsif($winapi->name eq "win32") {
-	if($calling_convention =~ /^__cdecl$/) {
+	if($calling_convention eq "__cdecl") {
 	    $implemented_calling_convention = "cdecl";
-	} elsif($calling_convention =~ /^VFWAPIV|WINAPIV$/) {
+	} elsif($calling_convention =~ /^(?:VFWAPIV|WINAPIV)$/) {
 	    $implemented_calling_convention = "varargs";
-	} elsif($calling_convention =~ /^__stdcall|VFWAPI|WINAPI|CALLBACK$/) {
-	    if(defined($implemented_return_kind) && $implemented_return_kind =~ /^longlong$/) {
+	} elsif($calling_convention =~ /^(?:__stdcall|VFWAPI|WINAPI|CALLBACK)$/) {
+	    if(defined($implemented_return_kind) && $implemented_return_kind eq "longlong") {
 		$implemented_calling_convention = "stdcall"; # FIXME: Check entry flags
 	    } else {
 		$implemented_calling_convention = "stdcall";
 	    }
-	} elsif($calling_convention =~ /^__asm$/) {
+	} elsif($calling_convention eq "__asm") {
     	    $implemented_calling_convention = "asm";
 	} else {
 	    $implemented_calling_convention = "cdecl";
@@ -166,7 +167,7 @@
     elsif($implemented_calling_convention ne $declared_calling_convention &&
        $implemented_calling_convention ne "asm" &&
        !($declared_calling_convention =~ /^pascal/ && $forbidden_return_type) &&
-       !($implemented_calling_convention =~ /^cdecl|varargs$/ && $declared_calling_convention =~ /^cdecl|varargs$/))
+       !($implemented_calling_convention =~ /^(?:cdecl|varargs)$/ && $declared_calling_convention =~ /^(?:cdecl|varargs)$/))
     {
 	if($options->calling_convention && (
             ($options->calling_convention_win16 && $winapi->name eq "win16") ||
@@ -203,7 +204,7 @@
 	$#argument_types--;
     }
 
-    if($internal_name =~ /^NTDLL__ftol|NTDLL__CIpow$/) { # FIXME: Kludge
+    if($internal_name =~ /^(?:NTDLL__ftol|NTDLL__CIpow)$/) { # FIXME: Kludge
 	# ignore
     } else {
 	my $n = 0;
@@ -230,7 +231,7 @@
 	    if(defined($kind) && $kind eq "struct16") {
 		$n+=4;
 		("long", "long", "long", "long");
-	    } elsif(defined($kind) && $kind =~ /^(?:longlong)$/) {
+	    } elsif(defined($kind) && $kind eq "longlong") {
 		$n+=2;
 		("long", "long");
 	    } else {
@@ -246,8 +247,8 @@
 	for my $n (0..$#argument_kinds) {
 	    if(!defined($argument_kinds[$n]) || !defined($declared_argument_kinds[$n])) { next; }
 
-	    if($argument_kinds[$n] =~ /^segptr|segstr$/ ||
-	       $declared_argument_kinds[$n] =~ /^segptr|segstr$/)
+	    if($argument_kinds[$n] =~ /^seg[ps]tr$/ ||
+	       $declared_argument_kinds[$n] =~ /^seg[ps]tr$/)
 	    {
 		$segmented = 1;
 	    }
@@ -334,10 +335,10 @@
 	    my $called_name = $1;
 	    my $channel = $2;
 	    my $called_arguments = $3;
-	    if($called_name =~ /^if|for|while|switch|sizeof$/) {
+	    if($called_name =~ /^(?:if|for|while|switch|sizeof)$/) {
 		# Nothing
-	    } elsif($called_name =~ /^ERR|FIXME|MSG|TRACE|WARN$/) {
-		if($first_debug_message && $called_name =~ /^FIXME|TRACE$/) {
+	    } elsif($called_name =~ /^(?:ERR|FIXME|MSG|TRACE|WARN)$/) {
+		if($first_debug_message && $called_name =~ /^(?:FIXME|TRACE)$/) {
 		    $first_debug_message = 0;
 		    if($called_arguments =~ /^\"\((.*?)\)(.*?)\"\s*,\s*(.*?)$/) {
 			my $formating = $1;


More information about the wine-patches mailing list