r5672: Use switch_type() and the token storage mechanism for unions:

 - Makes union handling less special
 - Allows unions in arrays, etc
 - Compatible with midl
 - Pidl will warn about switch_type() and the type of the switch_is() variable being different
(This used to be commit dc6b4ffc82a191631bc16a4b93a4916a39183ec6)

4 files changed, 83 insertions, 50 deletions

diff --git a/source4/build/pidl/ndr.pm b/source4/build/pidl/ndr.pm
index 333f20e78f..ac65f16d95 100644
--- a/source4/build/pidl/ndr.pm
+++ b/source4/build/pidl/ndr.pm
@@ -666,9 +666,11 @@ sub ParseElementPushScalar($$$)
 		ParseArrayPush($e, $ndr, "r->", "NDR_SCALARS");
 	} elsif (need_alloc($e)) {
 		# no scalar component
-	} elsif (my $switch = util::has_property($e, "switch_is")) {
-		ParseSwitchPush($e, $ndr, $var_prefix, $ndr_flags, $switch);
 	} else {
+		if (my $switch = util::has_property($e, "switch_is")) {
+			ParseSwitchPush($e, $ndr, $var_prefix, $ndr_flags, $switch);
+		}
+
 		pidl "NDR_CHECK(ndr_push_$e->{TYPE}($ndr, $ndr_flags, $cprefix$var_prefix$e->{NAME}));";
 	}
 
@@ -759,33 +761,8 @@ sub ParseSwitchPull($$$$$)
 
 	check_null_pointer($switch_var);
 
-	if (!defined $utype ||
-	    !util::has_property($utype, "nodiscriminant")) {
-		my $e2 = util::find_sibling($e, $switch);
-		my $type_decl = typelist::mapType($e2);
-		pidl "if (($ndr_flags) & NDR_SCALARS) {";
-		indent;
-		pidl "$type_decl _level;";
-		pidl "NDR_CHECK(ndr_pull_$e2->{TYPE}($ndr, NDR_SCALARS, &_level));";
-		if ($switch_var =~ /r->in/) {
-			pidl "if (!($ndr->flags & LIBNDR_FLAG_REF_ALLOC) && _level != $switch_var) {";
-			indent;
-		} else {
-			pidl "if (_level != $switch_var) {"; 
-			indent;
-		}
-		pidl "return ndr_pull_error($ndr, NDR_ERR_BAD_SWITCH, \"Bad switch value %u in $e->{NAME}\", _level);";
-		deindent;
-		if ($switch_var =~ /r->/) {
-			pidl "} else { $switch_var = _level; }";
-		} else {
-			pidl "}";
-		}
-		deindent;
-		pidl "}";
-	}
+	pidl "NDR_CHECK(ndr_pull_set_switch_value($ndr, $cprefix$var_prefix$e->{NAME}, $switch_var));";
 
-	pidl "NDR_CHECK(ndr_pull_$e->{TYPE}($ndr, $ndr_flags, $switch_var, $cprefix$var_prefix$e->{NAME}));";
 }
 
 #####################################################################
@@ -802,16 +779,8 @@ sub ParseSwitchPush($$$$$)
 
 	check_null_pointer($switch_var);
 
-	my $utype = typelist::getType($e->{TYPE});
-	if (!defined $utype ||
-	    !util::has_property($utype, "nodiscriminant")) {
-		my $e2 = util::find_sibling($e, $switch);
-		pidl "if (($ndr_flags) & NDR_SCALARS) {";
-		pidl "\tNDR_CHECK(ndr_push_$e2->{TYPE}($ndr, NDR_SCALARS, $switch_var));";
-		pidl "}";
-	}
+	pidl "NDR_CHECK(ndr_push_set_switch_value($ndr, $cprefix$var_prefix$e->{NAME}, $switch_var));";
 
-	pidl "NDR_CHECK(ndr_push_$e->{TYPE}($ndr, $ndr_flags, $switch_var, $cprefix$var_prefix$e->{NAME}));";
 }
 
 #####################################################################
@@ -838,9 +807,11 @@ sub ParseElementPullScalar($$$)
 	} elsif (need_wire_pointer($e)) {
 		ParsePtrPull($e, $ptr_prefix.$var_prefix);
 	} elsif (is_surrounding_array($e)) {
-	} elsif (my $switch = util::has_property($e, "switch_is")) {
-		ParseSwitchPull($e, $ndr, $var_prefix, $ndr_flags, $switch);
 	} else {
+		if (my $switch = util::has_property($e, "switch_is")) {
+			ParseSwitchPull($e, $ndr, $var_prefix, $ndr_flags, $switch);
+		}
+
 		pidl "NDR_CHECK(ndr_pull_$e->{TYPE}($ndr, $ndr_flags, $cprefix$var_prefix$e->{NAME}));";
 	}
 
@@ -919,9 +890,11 @@ sub ParseElementPushBuffer($$)
 	    
 	if (util::array_size($e)) {
 		ParseArrayPush($e, $ndr, "r->", $ndr_flags);
-	} elsif (my $switch = util::has_property($e, "switch_is")) {
-		ParseSwitchPush($e, $ndr, $var_prefix, $ndr_flags, $switch);
 	} else {
+		if (my $switch = util::has_property($e, "switch_is")) {
+			ParseSwitchPush($e, $ndr, $var_prefix, $ndr_flags, $switch);
+		}
+
 		pidl "NDR_CHECK(ndr_push_$e->{TYPE}(ndr, $ndr_flags, $cprefix$var_prefix$e->{NAME}));";
 	}
 
@@ -980,9 +953,11 @@ sub ParseElementPullBuffer($$)
 
 	if (util::array_size($e)) {
 		ParseArrayPull($e, $ndr, "r->", $ndr_flags);
-	} elsif (my $switch = util::has_property($e, "switch_is")) {
-		ParseSwitchPull($e, $ndr, $var_prefix, $ndr_flags, $switch);
 	} else {
+		if (my $switch = util::has_property($e, "switch_is")) {
+			ParseSwitchPull($e, $ndr, $var_prefix, $ndr_flags, $switch);
+		}
+
 		pidl "NDR_CHECK(ndr_pull_$e->{TYPE}($ndr, $ndr_flags, $cprefix$var_prefix$e->{NAME}));";
 	}
 
@@ -1400,10 +1375,20 @@ sub ParseUnionPush($)
 	my $e = shift;
 	my $have_default = 0;
 
+	pidl "int level;";
+
 	start_flags($e);
 
+	pidl "level = ndr_push_get_switch_value(ndr, r);";
+
 	pidl "if (!(ndr_flags & NDR_SCALARS)) goto buffers;";
 
+	if (!util::has_property($e, "nodiscriminant")) {
+		my $switch_type = util::has_property($e, "switch_type");
+		$switch_type = "uint32" unless  (defined ($switch_type));
+		pidl "NDR_CHECK(ndr_push_$switch_type(ndr, NDR_SCALARS, level));";
+	}
+
 	pidl "NDR_CHECK(ndr_push_struct_start(ndr));";
 
 #	my $align = union_alignment($e);
@@ -1506,11 +1491,30 @@ sub ParseUnionPull($)
 {
 	my $e = shift;
 	my $have_default = 0;
+	my $switch_type = util::has_property($e, "switch_type");
+	$switch_type = "uint32" unless defined($switch_type);
+
+	pidl "int level;";
+	if (!util::has_property($e, "nodiscriminant")) {
+		if (typelist::typeIs($switch_type, "ENUM")) {
+			$switch_type = typelist::enum_type_fn(typelist::getType($switch_type));
+		}
+		pidl typelist::mapScalarType($switch_type) . " _level;";
+	}
 
 	start_flags($e);
 
+	pidl "level = ndr_pull_get_switch_value(ndr, r);";
+
 	pidl "if (!(ndr_flags & NDR_SCALARS)) goto buffers;";
 
+	if (!util::has_property($e, "nodiscriminant")) {
+		pidl "NDR_CHECK(ndr_pull_$switch_type(ndr, NDR_SCALARS, &_level));";
+		pidl "if (_level != level) {"; 
+		pidl "\treturn ndr_pull_error(ndr, NDR_ERR_BAD_SWITCH, \"Bad switch value %u for $e->{PARENT}->{NAME}\", _level);";
+		pidl "}";
+	}
+
 	pidl "NDR_CHECK(ndr_pull_struct_start(ndr));";
 
 #	my $align = union_alignment($e);
@@ -1574,7 +1578,7 @@ sub ParseUnionPull($)
 sub ArgsUnionPush($)
 {
 	my $e = shift;
-	return "struct ndr_push *ndr, int ndr_flags, int level, union $e->{NAME} *r";
+	return "struct ndr_push *ndr, int ndr_flags, union $e->{NAME} *r";
 }
 
 sub ArgsUnionPrint($)
@@ -1586,7 +1590,7 @@ sub ArgsUnionPrint($)
 sub ArgsUnionPull($)
 {
 	my $e = shift;
-	return "struct ndr_pull *ndr, int ndr_flags, int level, union $e->{NAME} *r";
+	return "struct ndr_pull *ndr, int ndr_flags, union $e->{NAME} *r";
 }
 
 sub ArgsUnionNdrSize($)
diff --git a/source4/build/pidl/pidl.pl b/source4/build/pidl/pidl.pl
index 6bbb2ec78c..9e1ce48467 100755
--- a/source4/build/pidl/pidl.pl
+++ b/source4/build/pidl/pidl.pl
@@ -137,6 +137,7 @@ sub process_file($)
 	if ($opt_parse) {
 		$pidl = IdlParse($idl_file);
 		defined @$pidl || die "Failed to parse $idl_file";
+		typelist::LoadIdl($pidl);
 		IdlValidator::Validate($pidl);
 		if ($opt_keep && !util::SaveStructure($pidl_file, $pidl)) {
 			    die "Failed to save $pidl_file\n";
@@ -157,10 +158,6 @@ sub process_file($)
 		unlink($tempfile);
 	}
 
-	if ($opt_header || $opt_parser || $opt_com_header || $opt_dcom_proxy) {
-		typelist::LoadIdl($pidl);
-	}
-
 	if ($opt_com_header) {
 		my $res = COMHeader::Parse($pidl);
 		if ($res) {
diff --git a/source4/build/pidl/typelist.pm b/source4/build/pidl/typelist.pm
index 2d38d1f18d..432497f9f6 100644
--- a/source4/build/pidl/typelist.pm
+++ b/source4/build/pidl/typelist.pm
@@ -22,6 +22,15 @@ sub getType($)
 	return $typedefs{$t};
 }
 
+sub typeIs($$)
+{
+	my $t = shift;
+	my $tt = shift;
+
+	return 1 if (hasType($t) and getType($t)->{DATA}->{TYPE} eq $tt);
+	return 0;
+}
+
 sub hasType($)
 {
 	my $t = shift;
diff --git a/source4/build/pidl/validator.pm b/source4/build/pidl/validator.pm
index 1636a155e3..bbef008ee5 100644
--- a/source4/build/pidl/validator.pm
+++ b/source4/build/pidl/validator.pm
@@ -45,6 +45,25 @@ sub ValidElement($)
 		fatal(el_name($e) . " : pidl does not support full NDR pointers yet\n");
 	}
 
+	# Check whether switches are used correctly.
+	if (my $switch = util::has_property($e, "switch_is")) {
+		my $e2 = util::find_sibling($e, $switch);
+		my $type = typelist::getType($e->{TYPE});
+
+		if (defined($type) and $type->{DATA}->{TYPE} ne "UNION") {
+			fatal(el_name($e) . ": switch_is() used on non-union type $e->{TYPE} which is a $type->{DATA}->{TYPE}");
+		}
+
+		if (!util::has_property($type, "nodiscriminant") and defined($e2)) {
+			my $discriminator_type = util::has_property($type, "switch_type");
+			$discriminator_type = "uint32" unless defined ($discriminator_type);
+
+			if ($e2->{TYPE} ne $discriminator_type) {
+				print el_name($e) . ": Warning: switch_is() is of type $e2->{TYPE}, while discriminator type for union $type->{NAME} is $discriminator_type\n";
+			}
+		}
+	}
+
 	if (util::has_property($e, "size_is") and not defined ($e->{ARRAY_LEN})) {
 		fatal(el_name($e) . " : size_is() on non-array element");
 	}
@@ -53,7 +72,6 @@ sub ValidElement($)
 		fatal(el_name($e) . " : length_is() on non-array element");
 	}
 
-	
 	if (!$e->{POINTERS} && (
 		util::has_property($e, "ptr") or
 		util::has_property($e, "unique") or
@@ -84,6 +102,11 @@ sub ValidStruct($)
 sub ValidUnion($)
 {
 	my($union) = shift;
+
+	if (util::has_property($union->{PARENT}, "nodiscriminant") and util::has_property($union->{PARENT}, "switch_type")) {
+		fatal($union->{PARENT}->{NAME} . ": switch_type() on union without discriminant");
+	}
+	
 	foreach my $e (@{$union->{ELEMENTS}}) {
 		$e->{PARENT} = $union;