Author: jelmer Date: 2007-01-09 23:41:25 +0000 (Tue, 09 Jan 2007) New Revision: 20637
WebSVN: http://websvn.samba.org/cgi-bin/viewcvs.cgi?view=rev&root=samba&rev=20637 Log: Don't check for NULL pointers when the pointer is guaranteed to not be NULL (if it is a ref pointer). Added: branches/SAMBA_4_0/source/pidl/tests/samba-ndr.pl Modified: branches/SAMBA_4_0/source/pidl/lib/Parse/Pidl/Samba4/NDR/Parser.pm branches/SAMBA_4_0/source/pidl/lib/Parse/Pidl/Util.pm Changeset: Modified: branches/SAMBA_4_0/source/pidl/lib/Parse/Pidl/Samba4/NDR/Parser.pm =================================================================== --- branches/SAMBA_4_0/source/pidl/lib/Parse/Pidl/Samba4/NDR/Parser.pm 2007-01-09 20:04:46 UTC (rev 20636) +++ branches/SAMBA_4_0/source/pidl/lib/Parse/Pidl/Samba4/NDR/Parser.pm 2007-01-09 23:41:25 UTC (rev 20637) @@ -10,12 +10,14 @@ require Exporter; @ISA = qw(Exporter); @EXPORT = qw(is_charset_array); [EMAIL PROTECTED] = qw(check_null_pointer); use strict; use Parse::Pidl::Typelist qw(hasType getType mapType); -use Parse::Pidl::Util qw(has_property ParseExpr print_uuid); +use Parse::Pidl::Util qw(has_property ParseExpr ParseExprExt print_uuid); use Parse::Pidl::NDR qw(GetPrevLevel GetNextLevel ContainsDeferred); use Parse::Pidl::Samba4 qw(is_intree choose_header); +use Parse::Pidl qw(warning); use vars qw($VERSION); $VERSION = '0.01'; @@ -165,29 +167,6 @@ } ##################################################################### -# check that a variable we get from ParseExpr isn't a null pointer -sub check_null_pointer($) -{ - my $size = shift; - if ($size =~ /^\*/) { - my $size2 = substr($size, 1); - pidl "if ($size2 == NULL) return NT_STATUS_INVALID_PARAMETER_MIX;"; - } -} - -##################################################################### -# check that a variable we get from ParseExpr isn't a null pointer, -# putting the check at the end of the structure/function -sub check_null_pointer_deferred($) -{ - my $size = shift; - if ($size =~ /^\*/) { - my $size2 = substr($size, 1); - defer "if ($size2 == NULL) return NT_STATUS_INVALID_PARAMETER_MIX;"; - } -} - -##################################################################### # declare a function public or static, depending on its attributes sub fn_declare($$$) { @@ -325,6 +304,61 @@ return $length; } +sub check_null_pointer($$$) +{ + my ($element, $env, $print_fn) = @_; + + return sub ($) { + my $expandedvar = shift; + my $check = 0; + + # Figure out the number of pointers in $ptr + $expandedvar =~ s/^(\**)//; + my $ptr = $1; + + my $var = undef; + foreach (keys %$env) { + if ($env->{$_} eq $expandedvar) { + $var = $_; + last; + } + } + + if (defined($var)) { + my $e; + # lookup ptr in $e + foreach (@{$element->{PARENT}->{ELEMENTS}}) { + if ($_->{NAME} eq $var) { + $e = $_; + last; + } + } + + $e or die("Environment doesn't match siblings"); + + # See if pointer at pointer level $level + # needs to be checked. + foreach my $l (@{$e->{LEVELS}}) { + if ($l->{TYPE} eq "POINTER" and + $l->{POINTER_INDEX} == length($ptr)) { + # No need to check ref pointers + $check = ($l->{POINTER_TYPE} ne "ref"); + last; + } + + if ($l->{TYPE} eq "DATA") { + warning($element, "too much dereferences for `$var'"); + } + } + } else { + warning($element, "unknown dereferenced expression `$expandedvar'"); + $check = 1; + } + + $print_fn->("if ($ptr$expandedvar == NULL) return NT_STATUS_INVALID_PARAMETER_MIX;") if $check; + } +} + ##################################################################### # parse an array - pull side sub ParseArrayPullHeader($$$$$) @@ -339,21 +373,19 @@ } elsif ($l->{IS_ZERO_TERMINATED}) { # Noheader arrays $length = $size = "ndr_get_string_size($ndr, sizeof(*$var_name))"; } else { - $length = $size = ParseExpr($l->{SIZE_IS}, $env, $e); + $length = $size = ParseExprExt($l->{SIZE_IS}, $env, $e, + check_null_pointer($e, $env, \&pidl)); } if ((!$l->{IS_SURROUNDING}) and $l->{IS_CONFORMANT}) { pidl "NDR_CHECK(ndr_pull_array_size(ndr, " . get_pointer_to($var_name) . "));"; } - if ($l->{IS_VARYING}) { pidl "NDR_CHECK(ndr_pull_array_length($ndr, " . get_pointer_to($var_name) . "));"; $length = "ndr_get_array_length($ndr, " . get_pointer_to($var_name) .")"; } - check_null_pointer($length); - if ($length ne $size) { pidl "if ($length > $size) {"; indent; @@ -363,20 +395,18 @@ } if ($l->{IS_CONFORMANT} and not $l->{IS_ZERO_TERMINATED}) { - my $size = ParseExpr($l->{SIZE_IS}, $env, $e); defer "if ($var_name) {"; defer_indent; - check_null_pointer_deferred($size); + my $size = ParseExprExt($l->{SIZE_IS}, $env, $e, check_null_pointer($e, $env, \&defer)); defer "NDR_CHECK(ndr_check_array_size(ndr, (void*)" . get_pointer_to($var_name) . ", $size));"; defer_deindent; defer "}"; } if ($l->{IS_VARYING} and not $l->{IS_ZERO_TERMINATED}) { - my $length = ParseExpr($l->{LENGTH_IS}, $env, $e); defer "if ($var_name) {"; defer_indent; - check_null_pointer_deferred($length); + my $length = ParseExprExt($l->{LENGTH_IS}, $env, $e, check_null_pointer($e, $env, \&defer)); defer "NDR_CHECK(ndr_check_array_length(ndr, (void*)" . get_pointer_to($var_name) . ", $length));"; defer_deindent; defer "}" @@ -661,7 +691,7 @@ my ($e,$l,$var_name) = @_; if ($l->{POINTER_TYPE} eq "ref") { - check_null_pointer(get_value_of($var_name)); + pidl "if ($var_name == NULL) return NT_STATUS_INVALID_PARAMETER_MIX;"; if ($l->{LEVEL} eq "EMBEDDED") { pidl "NDR_CHECK(ndr_push_ref_ptr(ndr));"; } @@ -774,10 +804,8 @@ sub ParseSwitchPull($$$$$$) { my($e,$l,$ndr,$var_name,$ndr_flags,$env) = @_; - my $switch_var = ParseExpr($l->{SWITCH_IS}, $env, $e); + my $switch_var = ParseExprExt($l->{SWITCH_IS}, $env, $e, check_null_pointer($e, $env, \&pidl)); - check_null_pointer($switch_var); - $var_name = get_pointer_to($var_name); pidl "NDR_CHECK(ndr_pull_set_switch_value($ndr, $var_name, $switch_var));"; } @@ -787,9 +815,8 @@ sub ParseSwitchPush($$$$$$) { my($e,$l,$ndr,$var_name,$ndr_flags,$env) = @_; - my $switch_var = ParseExpr($l->{SWITCH_IS}, $env, $e); + my $switch_var = ParseExprExt($l->{SWITCH_IS}, $env, $e, check_null_pointer($e, $env, \&pidl)); - check_null_pointer($switch_var); $var_name = get_pointer_to($var_name); pidl "NDR_CHECK(ndr_push_set_switch_value($ndr, $var_name, $switch_var));"; } @@ -2014,7 +2041,6 @@ my $var = ParseExpr($e->{NAME}, $env, $e); - check_null_pointer($size); my $pl = GetPrevLevel($e, $l); if (defined($pl) and $pl->{TYPE} eq "POINTER" and @@ -2093,8 +2119,7 @@ and $e->{LEVELS}[1]->{IS_ZERO_TERMINATED}); if ($e->{LEVELS}[1]->{TYPE} eq "ARRAY") { - my $size = ParseExpr($e->{LEVELS}[1]->{SIZE_IS}, $env, $e); - check_null_pointer($size); + my $size = ParseExprExt($e->{LEVELS}[1]->{SIZE_IS}, $env, $e, check_null_pointer($e, $env, \&pidl)); pidl "NDR_PULL_ALLOC_N(ndr, r->out.$e->{NAME}, $size);"; Modified: branches/SAMBA_4_0/source/pidl/lib/Parse/Pidl/Util.pm =================================================================== --- branches/SAMBA_4_0/source/pidl/lib/Parse/Pidl/Util.pm 2007-01-09 20:04:46 UTC (rev 20636) +++ branches/SAMBA_4_0/source/pidl/lib/Parse/Pidl/Util.pm 2007-01-09 23:41:25 UTC (rev 20637) @@ -6,7 +6,7 @@ require Exporter; @ISA = qw(Exporter); [EMAIL PROTECTED] = qw(has_property property_matches ParseExpr is_constant make_str print_uuid MyDumper); [EMAIL PROTECTED] = qw(has_property property_matches ParseExpr ParseExprExt is_constant make_str print_uuid MyDumper); use vars qw($VERSION); $VERSION = '0.01'; @@ -115,4 +115,21 @@ undef); } +sub ParseExprExt($$$$) +{ + my($expr, $varlist, $e, $deref) = @_; + + die("Undefined value in ParseExpr") if not defined($expr); + + my $x = new Parse::Pidl::Expr(); + + return $x->Run($expr, sub { my $x = shift; error($e, $x); }, + # Lookup fn + sub { my $x = shift; + return($varlist->{$x}) if (defined($varlist->{$x})); + return $x; + }, + $deref); +} + 1; Added: branches/SAMBA_4_0/source/pidl/tests/samba-ndr.pl =================================================================== --- branches/SAMBA_4_0/source/pidl/tests/samba-ndr.pl 2007-01-09 20:04:46 UTC (rev 20636) +++ branches/SAMBA_4_0/source/pidl/tests/samba-ndr.pl 2007-01-09 23:41:25 UTC (rev 20637) @@ -0,0 +1,135 @@ +#!/usr/bin/perl +# (C) 2007 Jelmer Vernooij <[EMAIL PROTECTED]> +# Published under the GNU General Public License +use strict; +use warnings; + +use Test::More tests => 10; +use FindBin qw($RealBin); +use lib "$RealBin"; +use Util; +use Parse::Pidl::Util qw(MyDumper); +use Parse::Pidl::Samba4::NDR::Parser qw(check_null_pointer); + +my $output; +sub print_fn($) { my $x = shift; $output.=$x; } + +# Test case 1: Simple unique pointer dereference + +$output = ""; +my $fn = check_null_pointer({ + PARENT => { + ELEMENTS => [ + { + NAME => "bla", + LEVELS => [ + { TYPE => "POINTER", + POINTER_INDEX => 0, + POINTER_TYPE => "unique" }, + { TYPE => "DATA" } + ], + }, + ] + } +}, { bla => "r->in.bla" }, \&print_fn); + + +test_warnings("", sub { $fn->("r->in.bla"); }); + +is($output, "if (r->in.bla == NULL) return NT_STATUS_INVALID_PARAMETER_MIX;"); + +# Test case 2: Simple ref pointer dereference + +$output = ""; +$fn = check_null_pointer({ + PARENT => { + ELEMENTS => [ + { + NAME => "bla", + LEVELS => [ + { TYPE => "POINTER", + POINTER_INDEX => 0, + POINTER_TYPE => "ref" }, + { TYPE => "DATA" } + ], + }, + ] + } +}, { bla => "r->in.bla" }, \&print_fn); + +test_warnings("", sub { $fn->("r->in.bla"); }); + +is($output, ""); + +# Test case 3: Illegal dereference + +$output = ""; +$fn = check_null_pointer({ + FILE => "nofile", + LINE => 1, + PARENT => { + ELEMENTS => [ + { + NAME => "bla", + LEVELS => [ + { TYPE => "DATA" } + ], + }, + ] + } +}, { bla => "r->in.bla" }, \&print_fn); + +test_warnings("nofile:1: too much dereferences for `bla'\n", + sub { $fn->("r->in.bla"); }); + +is($output, ""); + +# Test case 4: Double pointer dereference + +$output = ""; +$fn = check_null_pointer({ + PARENT => { + ELEMENTS => [ + { + NAME => "bla", + LEVELS => [ + { TYPE => "POINTER", + POINTER_INDEX => 0, + POINTER_TYPE => "unique" }, + { TYPE => "POINTER", + POINTER_INDEX => 1, + POINTER_TYPE => "unique" }, + { TYPE => "DATA" } + ], + }, + ] + } +}, { bla => "r->in.bla" }, \&print_fn); + +test_warnings("", + sub { $fn->("*r->in.bla"); }); + +is($output, "if (*r->in.bla == NULL) return NT_STATUS_INVALID_PARAMETER_MIX;"); + +# Test case 5: Unknown variable + +$output = ""; +$fn = check_null_pointer({ + FILE => "nofile", + LINE => 2, + PARENT => { + ELEMENTS => [ + { + NAME => "bla", + LEVELS => [ + { TYPE => "DATA" } + ], + }, + ] + } +}, { }, \&print_fn); + +test_warnings("nofile:2: unknown dereferenced expression `r->in.bla'\n", + sub { $fn->("r->in.bla"); }); + +is($output, "if (r->in.bla == NULL) return NT_STATUS_INVALID_PARAMETER_MIX;"); Property changes on: branches/SAMBA_4_0/source/pidl/tests/samba-ndr.pl ___________________________________________________________________ Name: svn:executable + *
