r464: a big improvement to the API for writing server-side RPC

servers. Previously the server pipe code needed to return the RPC
level status (nearly always "OK") and separately set the function call
return using r->out.result. All the programmers writing servers
(metze, jelmer and me) were often getting this wrong, by doing things
like "return NT_STATUS_NO_MEMORY" which was really quite meaningless
as there is no code like that at the dcerpc level.

I have now modified pidl to generate the necessary boilerplate so that
just returning the status you want from the function will work. So for
a NTSTATUS function you return NT_STATUS_XXX and from a WERROR
function you return WERR_XXX. If you really want to generate a DCERPC
level fault rather than just a return value in your function then you
should use the DCESRV_FAULT() macro which will correctly generate a
fault for you.

As a side effect, this also adds automatic type checking of all of our
server side rpc functions, which was impossible with the old API. When
I changed the API I found and fixed quite a few functions with the
wrong type information, so this is definately useful.

I have also changed the server side template generation to generate a
DCERPC "operation range error" by default when you have not yet filled
in a server side function. This allows us to correctly implement
functions in any order in our rpc pipe servers and give the client the
right information about the fault.
(This used to be commit a4df5c7cf88891a78d82c8d6d7f058d8485e73f0)

2 files changed, 42 insertions, 11 deletions

diff --git a/source4/build/pidl/server.pm b/source4/build/pidl/server.pm
index f8016b2923..67b1fda615 100644
--- a/source4/build/pidl/server.pm
+++ b/source4/build/pidl/server.pm
@@ -15,6 +15,30 @@ sub pidl($)
 	$res .= shift;
 }
 
+
+#####################################################
+# generate the switch statement for function dispatch
+sub gen_dispatch_switch($)
+{
+	my $data = shift;
+
+	my $count = 0;
+	foreach my $d (@{$data}) {
+		next if ($d->{TYPE} ne "FUNCTION");
+
+		pidl "\tcase $count: {\n";
+		pidl "\t\tstruct $d->{NAME} *r2 = r;\n";
+		if ($d->{RETURN_TYPE} && $d->{RETURN_TYPE} ne "void") {
+			pidl "\t\tr2->out.result = $d->{NAME}(dce_call, mem_ctx, r2);\n";
+		} else {
+			pidl "\t\t$d->{NAME}(dce_call, mem_ctx, r2);\n";
+		}
+		pidl "\t\tbreak;\n\t}\n";
+		$count++; 
+	}
+}
+
+
 #####################################################################
 # produce boilerplate code for a interface
 sub Boilerplate_Iface($)
@@ -33,14 +57,6 @@ sub Boilerplate_Iface($)
 		return;
 	}
 
-	pidl "static const dcesrv_dispatch_fn_t $name\_dispatch_table[] = {\n";
-	foreach my $d (@{$data}) {
-		if ($d->{TYPE} eq "FUNCTION") {
-			pidl "\t(dcesrv_dispatch_fn_t)$d->{NAME},\n";
-		}
-	}
-	pidl "\tNULL};\n\n";
-
 	pidl "
 static NTSTATUS $name\__op_bind(struct dcesrv_call_state *dce_call, const struct dcesrv_interface *iface)
 {
@@ -64,7 +80,22 @@ static NTSTATUS $name\__op_dispatch(struct dcesrv_call_state *dce_call, TALLOC_C
 {
 	uint16 opnum = dce_call->pkt.u.request.opnum;
 
-	return $name\_dispatch_table[opnum](dce_call, mem_ctx, r);	
+	dce_call->fault_code = 0;
+
+	switch (opnum) {
+";
+	gen_dispatch_switch($data);
+
+pidl "
+	default:
+		dce_call->fault_code = DCERPC_FAULT_OP_RNG_ERROR;
+		break;
+	}
+
+	if (dce_call->fault_code != 0) {
+		return NT_STATUS_NET_WRITE_FAULT;
+	}
+	return NT_STATUS_OK;
 }
 
 static const struct dcesrv_interface $name\_interface = {
diff --git a/source4/build/pidl/template.pm b/source4/build/pidl/template.pm
index 6e0decbf40..2942c6f713 100644
--- a/source4/build/pidl/template.pm
+++ b/source4/build/pidl/template.pm
@@ -52,10 +52,10 @@ sub Template($)
 /* 
   $fname 
 */
-static NTSTATUS $fname(struct dcesrv_call_state *dce_call, TALLOC_CTX *mem_ctx,
+static $d->{RETURN_TYPE} $fname(struct dcesrv_call_state *dce_call, TALLOC_CTX *mem_ctx,
 		       struct $fname *r)
 {
-	return NT_STATUS_NOT_IMPLEMENTED;
+	DCESRV_FAULT(DCERPC_FAULT_OP_RNG_ERROR);
 }
 
 ";