From ff02537261e53b4ec60e5dcad32bf4207065b028 Mon Sep 17 00:00:00 2001 From: Andrew Tridgell Date: Thu, 13 Nov 2003 09:23:58 +0000 Subject: I think we now handle conformant arrays in structures correctly - the test cases pass (This used to be commit 22e15023509f8f1682865d72765e79f41ab7d149) --- source4/build/pidl/NOTES.txt | 78 +++++++++++++++++++++++++++++++++++++++++ source4/build/pidl/header.pm | 8 ++--- source4/build/pidl/idl.gram | 5 ++- source4/build/pidl/parser.pm | 83 ++++++++++++++++++++++++++++++++++++-------- 4 files changed, 155 insertions(+), 19 deletions(-) create mode 100644 source4/build/pidl/NOTES.txt diff --git a/source4/build/pidl/NOTES.txt b/source4/build/pidl/NOTES.txt new file mode 100644 index 0000000000..7726f22ec8 --- /dev/null +++ b/source4/build/pidl/NOTES.txt @@ -0,0 +1,78 @@ +FIXED ARRAY +----------- + +A fixed array looks like this: + + typedef struct { + long s[4]; + } Struct1; + +the NDR representation looks just like 4 separate long +declarations. The array size is not encoded on the wire. + + +CONFORMANT ARRAYS +----------------- + +A conformant array is one with that ends in [*] or []. The strange +things about conformant arrays are: + + * they can only appear as the last element of a structure + + * the array size appears before the structure itself on the wire. + +So, in this example: + + typedef struct { + long abc; + long count; + long foo; + [size_is(count)] long s[*]; + } Struct1; + +it appears like this: + +[size_is] [abc] [count] [foo] [s...] + +the first [size_is] field is the allocation size of the array, and +occurs before the array elements and even before the structure +alignment. + +Note that size_is() can refer to a constant, but that doesn't change +the wire representation. It does not make the array a fixed array. + +midl.exe would write the above array as the following C header: + + typedef struct { + long abc; + long count; + long foo; + long s[1]; + } Struct1; + +VARYING ARRAYS +-------------- + +A varying array looks like this: + + typedef struct { + long abc; + long count; + long foo; + [size_is(count)] long *s; + } Struct1; + +This will look like this on the wire: + +[abc] [count] [foo] [PTR_s] [count] [s...] + + +VALIDATOR +--------- + +We need to write an IDL validator, so we know that we are writing +valid IDL. Right now the compiler sails on regardless in many cases +even if the IDL is invalid (for example, I don't check that conformant +arrays are always the last element in any structure). There are dozens +of rules that should be checked. + diff --git a/source4/build/pidl/header.pm b/source4/build/pidl/header.pm index 6e8842b1d7..53e495883d 100644 --- a/source4/build/pidl/header.pm +++ b/source4/build/pidl/header.pm @@ -59,13 +59,13 @@ sub HeaderElement($) $res .= "*"; } } - if (defined $element->{ARRAY_LEN} && - $element->{ARRAY_LEN} eq "*") { + if (defined $element->{ARRAY_LEN} && $element->{ARRAY_LEN} eq "*") { + # conformant arrays are ugly! I choose to implement them with + # pointers instead of the [1] method $res .= "*"; } $res .= "$element->{NAME}"; - if (defined $element->{ARRAY_LEN} && - $element->{ARRAY_LEN} ne "*") { + if (defined $element->{ARRAY_LEN} && $element->{ARRAY_LEN} ne "*") { $res .= "[$element->{ARRAY_LEN}]"; } $res .= ";\n"; diff --git a/source4/build/pidl/idl.gram b/source4/build/pidl/idl.gram index c08c6b0e81..956b3e9ff9 100644 --- a/source4/build/pidl/idl.gram +++ b/source4/build/pidl/idl.gram @@ -76,7 +76,10 @@ base_element: property_list(s?) type pointer(s?) identifier array_len(?) }} | -array_len: '[' constant ']' +array_len: + '[]' + { "*" } + | '[' constant ']' { $item{constant} } | diff --git a/source4/build/pidl/parser.pm b/source4/build/pidl/parser.pm index eb7758b1cc..9c987a9f85 100644 --- a/source4/build/pidl/parser.pm +++ b/source4/build/pidl/parser.pm @@ -81,16 +81,18 @@ sub ParseArrayPush($$) my $e = shift; my $var_prefix = shift; my $size = find_size_var($e, util::array_size($e)); - my $const = ""; - if (util::is_constant($size)) { - $const = "_const"; + if (defined $e->{CONFORMANT_SIZE}) { + # the conformant size has already been pushed + } elsif (!util::is_constant($size)) { + # we need to emit the array size + $res .= "\t\tNDR_CHECK(ndr_push_uint32(ndr, $size));\n"; } if (util::is_scalar_type($e->{TYPE})) { - $res .= "\t\tNDR_CHECK(ndr_push$const\_array_$e->{TYPE}(ndr, $var_prefix$e->{NAME}, $size));\n"; + $res .= "\t\tNDR_CHECK(ndr_push_array_$e->{TYPE}(ndr, $var_prefix$e->{NAME}, $size));\n"; } else { - $res .= "\t\tNDR_CHECK(ndr_push$const\_array(ndr, ndr_flags, $var_prefix$e->{NAME}, sizeof($var_prefix$e->{NAME}\[0]), $size, (ndr_push_flags_fn_t)ndr_push_$e->{TYPE}));\n"; + $res .= "\t\tNDR_CHECK(ndr_push_array(ndr, ndr_flags, $var_prefix$e->{NAME}, sizeof($var_prefix$e->{NAME}\[0]), $size, (ndr_push_flags_fn_t)ndr_push_$e->{TYPE}));\n"; } } @@ -116,19 +118,39 @@ sub ParseArrayPull($$) my $e = shift; my $var_prefix = shift; my $size = find_size_var($e, util::array_size($e)); - my $const = ""; + my $alloc_size = $size; - if (util::is_constant($size)) { - $const = "_const"; + # if this is a conformant array then we use that size to allocate, and make sure + # we allocate enough to pull the elements + if (defined $e->{CONFORMANT_SIZE}) { + $alloc_size = $e->{CONFORMANT_SIZE}; + + $res .= "\tif ($size > $alloc_size) {\n"; + $res .= "\t\treturn ndr_pull_error(ndr, NDR_ERR_CONFORMANT_SIZE, \"Bad conformant size %u should be %u\", $alloc_size, $size);\n"; + $res .= "\t}\n"; + } elsif (!util::is_constant($size)) { + # non fixed arrays encode the size just before the array + $res .= "\t{\n"; + $res .= "\t\tuint32 _array_size;\n"; + $res .= "\t\tNDR_CHECK(ndr_pull_uint32(ndr, &_array_size));\n"; + $res .= "\t\tif ($size > _array_size) {\n"; + $res .= "\t\t\treturn ndr_pull_error(ndr, NDR_ERR_ARRAY_SIZE, \"Bad array size %u should be %u\", _array_size, $size);\n"; + $res .= "\t\t}\n"; + $res .= "\t}\n"; } if (util::need_alloc($e) && !util::is_constant($size)) { - $res .= "\t\tNDR_ALLOC_N_SIZE(ndr, $var_prefix$e->{NAME}, $size, sizeof($var_prefix$e->{NAME}\[0]));\n"; + $res .= "\t\tNDR_ALLOC_N_SIZE(ndr, $var_prefix$e->{NAME}, $alloc_size, sizeof($var_prefix$e->{NAME}\[0]));\n"; + } + + if (util::has_property($e, "length_is")) { + die "we don't handle varying arrays yet"; } + if (util::is_scalar_type($e->{TYPE})) { - $res .= "\t\tNDR_CHECK(ndr_pull$const\_array_$e->{TYPE}(ndr, $var_prefix$e->{NAME}, $size));\n"; + $res .= "\t\tNDR_CHECK(ndr_pull_array_$e->{TYPE}(ndr, $var_prefix$e->{NAME}, $size));\n"; } else { - $res .= "\t\tNDR_CHECK(ndr_pull$const\_array(ndr, ndr_flags, (void **)$var_prefix$e->{NAME}, sizeof($var_prefix$e->{NAME}\[0]), $size, (ndr_pull_flags_fn_t)ndr_pull_$e->{TYPE}));\n"; + $res .= "\t\tNDR_CHECK(ndr_pull_array(ndr, ndr_flags, (void **)$var_prefix$e->{NAME}, sizeof($var_prefix$e->{NAME}\[0]), $size, (ndr_pull_flags_fn_t)ndr_pull_$e->{TYPE}));\n"; } } @@ -191,7 +213,7 @@ sub ParseElementPullSwitch($$$$) $res .= "\t{ uint16 _level;\n"; $res .= "\tNDR_CHECK(ndr_pull_$e->{TYPE}(ndr, $ndr_flags, &_level, $cprefix$var_prefix$e->{NAME}));\n"; - $res .= "\tif (_level != $switch_var) return NT_STATUS_INVALID_LEVEL;\n"; + $res .= "\tif (_level != $switch_var) return ndr_pull_error(ndr, NDR_ERR_BAD_SWITCH, \"Bad switch value %u in $e->{NAME}\");\n"; $res .= "\t}\n"; } @@ -322,6 +344,7 @@ sub ParseStructPush($) { my($struct) = shift; my($struct_len); + my $conform_e; if (! defined $struct->{ELEMENTS}) { return; @@ -341,6 +364,19 @@ sub ParseStructPush($) $res .= "\tndr_push_save(ndr, &_save1);\n"; } + # see if the structure contains a conformant array. If it + # does, then it must be the last element of the structure, and + # we need to push the conformant length early, as it fits on + # the wire before the structure (and even before the structure + # alignment) + my $e = $struct->{ELEMENTS}[-1]; + if (defined $e->{ARRAY_LEN} && $e->{ARRAY_LEN} eq "*") { + my $size = find_size_var($e, util::array_size($e)); + $e->{CONFORMANT_SIZE} = $size; + $conform_e = $e; + $res .= "\tNDR_CHECK(ndr_push_uint32(ndr, $size));\n"; + } + my $align = struct_alignment($struct); $res .= "\tNDR_CHECK(ndr_push_align(ndr, $align));\n"; @@ -394,11 +430,24 @@ sub ParseStructPull($) { my($struct) = shift; my($struct_len); + my $conform_e; if (! defined $struct->{ELEMENTS}) { return; } + # see if the structure contains a conformant array. If it + # does, then it must be the last element of the structure, and + # we need to pull the conformant length early, as it fits on + # the wire before the structure (and even before the structure + # alignment) + my $e = $struct->{ELEMENTS}[-1]; + if (defined $e->{ARRAY_LEN} && $e->{ARRAY_LEN} eq "*") { + $conform_e = $e; + $res .= "\tuint32 _conformant_size;\n"; + $conform_e->{CONFORMANT_SIZE} = "_conformant_size"; + } + # declare any internal pointers we need foreach my $e (@{$struct->{ELEMENTS}}) { $e->{PARENT} = $struct; @@ -426,6 +475,10 @@ sub ParseStructPull($) $res .= "\tndr_pull_save(ndr, &_save);\n"; } + if (defined $conform_e) { + $res .= "\tNDR_CHECK(ndr_pull_uint32(ndr, &$conform_e->{CONFORMANT_SIZE}));\n"; + } + my $align = struct_alignment($struct); $res .= "\tNDR_CHECK(ndr_pull_align(ndr, $align));\n"; @@ -490,7 +543,8 @@ sub ParseUnionPull($) ParseElementPullScalar($el->{DATA}, "r->", "NDR_SCALARS"); $res .= "\tbreak;\n\n"; } - $res .= "\tdefault:\n\t\treturn NT_STATUS_INVALID_LEVEL;\n"; + $res .= "\tdefault:\n"; + $res .= "\t\treturn ndr_pull_error(ndr, NDR_ERR_BAD_SWITCH, \"Bad switch value %u in $e->{NAME}\", *level);\n"; $res .= "\t}\n"; $res .= "buffers:\n"; $res .= "\tif (!(ndr_flags & NDR_BUFFERS)) goto done;\n"; @@ -500,7 +554,8 @@ sub ParseUnionPull($) ParseElementPullBuffer($el->{DATA}, "r->", "NDR_BUFFERS"); $res .= "\tbreak;\n\n"; } - $res .= "\tdefault:\n\t\treturn NT_STATUS_INVALID_LEVEL;\n"; + $res .= "\tdefault:\n"; + $res .= "\t\treturn ndr_pull_error(ndr, NDR_ERR_BAD_SWITCH, \"Bad switch value %u in $e->{NAME}\", *level);\n"; $res .= "\t}\n"; $res .= "done:\n"; } -- cgit