From 348b7bc380e4ce95cf053134e62c3f5ab6520e34 Mon Sep 17 00:00:00 2001 From: Jelmer Vernooij Date: Wed, 10 Jan 2007 00:37:30 +0000 Subject: r20638: Check for NULL pointers (where possible) in print functions. Fixes #4218, but without reintroducing coverity warnings. (This used to be commit a0e2e30d570f246d646f88d7f81ab08208b96131) --- source4/pidl/lib/Parse/Pidl/Samba4/NDR/Parser.pm | 51 ++++++++++++------------ source4/pidl/tests/samba-ndr.pl | 16 ++++---- 2 files changed, 34 insertions(+), 33 deletions(-) diff --git a/source4/pidl/lib/Parse/Pidl/Samba4/NDR/Parser.pm b/source4/pidl/lib/Parse/Pidl/Samba4/NDR/Parser.pm index 49f9e051a8..2212044d3b 100644 --- a/source4/pidl/lib/Parse/Pidl/Samba4/NDR/Parser.pm +++ b/source4/pidl/lib/Parse/Pidl/Samba4/NDR/Parser.pm @@ -304,9 +304,9 @@ sub ParseArrayPushHeader($$$$$) return $length; } -sub check_null_pointer($$$) +sub check_null_pointer($$$$) { - my ($element, $env, $print_fn) = @_; + my ($element, $env, $print_fn, $return) = @_; return sub ($) { my $expandedvar = shift; @@ -355,7 +355,7 @@ sub check_null_pointer($$$) $check = 1; } - $print_fn->("if ($ptr$expandedvar == NULL) return NT_STATUS_INVALID_PARAMETER_MIX;") if $check; + $print_fn->("if ($ptr$expandedvar == NULL) $return") if $check; } } @@ -374,7 +374,7 @@ sub ParseArrayPullHeader($$$$$) $length = $size = "ndr_get_string_size($ndr, sizeof(*$var_name))"; } else { $length = $size = ParseExprExt($l->{SIZE_IS}, $env, $e, - check_null_pointer($e, $env, \&pidl)); + check_null_pointer($e, $env, \&pidl, "return NT_STATUS_INVALID_PARAMETER_MIX;")); } if ((!$l->{IS_SURROUNDING}) and $l->{IS_CONFORMANT}) { @@ -397,7 +397,7 @@ sub ParseArrayPullHeader($$$$$) if ($l->{IS_CONFORMANT} and not $l->{IS_ZERO_TERMINATED}) { defer "if ($var_name) {"; defer_indent; - my $size = ParseExprExt($l->{SIZE_IS}, $env, $e, check_null_pointer($e, $env, \&defer)); + my $size = ParseExprExt($l->{SIZE_IS}, $env, $e, check_null_pointer($e, $env, \&defer, "return NT_STATUS_INVALID_PARAMETER_MIX;")); defer "NDR_CHECK(ndr_check_array_size(ndr, (void*)" . get_pointer_to($var_name) . ", $size));"; defer_deindent; defer "}"; @@ -406,7 +406,7 @@ sub ParseArrayPullHeader($$$$$) if ($l->{IS_VARYING} and not $l->{IS_ZERO_TERMINATED}) { defer "if ($var_name) {"; defer_indent; - my $length = ParseExprExt($l->{LENGTH_IS}, $env, $e, check_null_pointer($e, $env, \&defer)); + my $length = ParseExprExt($l->{LENGTH_IS}, $env, $e, check_null_pointer($e, $env, \&defer, "return NT_STATUS_INVALID_PARAMETER_MIX;")); defer "NDR_CHECK(ndr_check_array_length(ndr, (void*)" . get_pointer_to($var_name) . ", $length));"; defer_deindent; defer "}" @@ -421,18 +421,16 @@ sub ParseArrayPullHeader($$$$$) sub compression_alg($$) { - my ($e,$l) = @_; - my $compression = $l->{COMPRESSION}; - my ($alg, $clen, $dlen) = split(/ /, $compression); + my ($e, $l) = @_; + my ($alg, $clen, $dlen) = split(/ /, $l->{COMPRESSION}); return $alg; } sub compression_clen($$$) { - my ($e,$l,$env) = @_; - my $compression = $l->{COMPRESSION}; - my ($alg, $clen, $dlen) = split(/ /, $compression); + my ($e, $l, $env) = @_; + my ($alg, $clen, $dlen) = split(/ /, $l->{COMPRESSION}); return ParseExpr($clen, $env, $e); } @@ -440,8 +438,7 @@ sub compression_clen($$$) sub compression_dlen($$$) { my ($e,$l,$env) = @_; - my $compression = $l->{COMPRESSION}; - my ($alg, $clen, $dlen) = split(/ /, $compression); + my ($alg, $clen, $dlen) = split(/ /, $l->{COMPRESSION}); return ParseExpr($dlen, $env, $e); } @@ -504,7 +501,7 @@ sub ParseSubcontextPushStart($$$$) { my ($e,$l,$ndr,$env) = @_; my $subndr = "_ndr_$e->{NAME}"; - my $subcontext_size = ParseExpr($l->{SUBCONTEXT_SIZE},$env, $e); + my $subcontext_size = ParseExpr($l->{SUBCONTEXT_SIZE}, $env, $e); pidl "{"; indent; @@ -522,7 +519,7 @@ sub ParseSubcontextPushEnd($$$$) { my ($e,$l,$ndr,$env) = @_; my $subndr = "_ndr_$e->{NAME}"; - my $subcontext_size = ParseExpr($l->{SUBCONTEXT_SIZE},$env, $e); + my $subcontext_size = ParseExpr($l->{SUBCONTEXT_SIZE}, $env, $e); if (defined $l->{COMPRESSION}) { ParseCompressionPushEnd($e, $l, $subndr, $env); @@ -537,7 +534,7 @@ sub ParseSubcontextPullStart($$$$) { my ($e,$l,$ndr,$env) = @_; my $subndr = "_ndr_$e->{NAME}"; - my $subcontext_size = ParseExpr($l->{SUBCONTEXT_SIZE},$env, $e); + my $subcontext_size = ParseExpr($l->{SUBCONTEXT_SIZE}, $env, $e); pidl "{"; indent; @@ -555,7 +552,7 @@ sub ParseSubcontextPullEnd($$$$) { my ($e,$l,$ndr,$env) = @_; my $subndr = "_ndr_$e->{NAME}"; - my $subcontext_size = ParseExpr($l->{SUBCONTEXT_SIZE},$env, $e); + my $subcontext_size = ParseExpr($l->{SUBCONTEXT_SIZE}, $env, $e); if (defined $l->{COMPRESSION}) { ParseCompressionPullEnd($e, $l, $subndr, $env); @@ -710,7 +707,7 @@ sub ParsePtrPush($$$) # print scalars in a structure element sub ParseElementPrint($$$) { - my($e,$var_name,$env) = @_; + my($e, $var_name, $env) = @_; return if (has_property($e, "noprint")); @@ -744,7 +741,8 @@ sub ParseElementPrint($$$) if ($l->{IS_ZERO_TERMINATED}) { $length = "ndr_string_length($var_name, sizeof(*$var_name))"; } else { - $length = ParseExpr($l->{LENGTH_IS}, $env, $e); + $length = ParseExprExt($l->{LENGTH_IS}, $env, $e, + check_null_pointer($e, $env, \&pidl, "return;")); } if (is_charset_array($e,$l)) { @@ -774,7 +772,8 @@ sub ParseElementPrint($$$) } pidl "ndr_print_$l->{DATA_TYPE}(ndr, \"$e->{NAME}\", $var_name);"; } elsif ($l->{TYPE} eq "SWITCH") { - my $switch_var = ParseExpr($l->{SWITCH_IS}, $env, $e); + my $switch_var = ParseExprExt($l->{SWITCH_IS}, $env, $e, + check_null_pointer($e, $env, \&pidl, "return;")); pidl "ndr_print_set_switch_value(ndr, " . get_pointer_to($var_name) . ", $switch_var);"; } } @@ -804,7 +803,8 @@ sub ParseElementPrint($$$) sub ParseSwitchPull($$$$$$) { my($e,$l,$ndr,$var_name,$ndr_flags,$env) = @_; - my $switch_var = ParseExprExt($l->{SWITCH_IS}, $env, $e, check_null_pointer($e, $env, \&pidl)); + my $switch_var = ParseExprExt($l->{SWITCH_IS}, $env, $e, + check_null_pointer($e, $env, \&pidl, "return NT_STATUS_INVALID_PARAMETER_MIX;")); $var_name = get_pointer_to($var_name); pidl "NDR_CHECK(ndr_pull_set_switch_value($ndr, $var_name, $switch_var));"; @@ -815,7 +815,8 @@ sub ParseSwitchPull($$$$$$) sub ParseSwitchPush($$$$$$) { my($e,$l,$ndr,$var_name,$ndr_flags,$env) = @_; - my $switch_var = ParseExprExt($l->{SWITCH_IS}, $env, $e, check_null_pointer($e, $env, \&pidl)); + my $switch_var = ParseExprExt($l->{SWITCH_IS}, $env, $e, + check_null_pointer($e, $env, \&pidl, "return NT_STATUS_INVALID_PARAMETER_MIX;")); $var_name = get_pointer_to($var_name); pidl "NDR_CHECK(ndr_push_set_switch_value($ndr, $var_name, $switch_var));"; @@ -1000,7 +1001,7 @@ sub ParseElementPullLevel ParseMemCtxPullStart($e,$l, $var_name); $var_name = get_value_of($var_name); - ParseElementPullLevel($e,GetNextLevel($e,$l), $ndr, $var_name, $env, 1, 1); + ParseElementPullLevel($e, GetNextLevel($e,$l), $ndr, $var_name, $env, 1, 1); ParseMemCtxPullEnd($e,$l); @@ -2119,7 +2120,7 @@ sub ParseFunctionPull($) and $e->{LEVELS}[1]->{IS_ZERO_TERMINATED}); if ($e->{LEVELS}[1]->{TYPE} eq "ARRAY") { - my $size = ParseExprExt($e->{LEVELS}[1]->{SIZE_IS}, $env, $e, check_null_pointer($e, $env, \&pidl)); + my $size = ParseExprExt($e->{LEVELS}[1]->{SIZE_IS}, $env, $e, check_null_pointer($e, $env, \&pidl, "return NT_STATUS_INVALID_PARAMETER_MIX;")); pidl "NDR_PULL_ALLOC_N(ndr, r->out.$e->{NAME}, $size);"; diff --git a/source4/pidl/tests/samba-ndr.pl b/source4/pidl/tests/samba-ndr.pl index 487f203f41..a3e94bd8b5 100755 --- a/source4/pidl/tests/samba-ndr.pl +++ b/source4/pidl/tests/samba-ndr.pl @@ -31,12 +31,12 @@ my $fn = check_null_pointer({ }, ] } -}, { bla => "r->in.bla" }, \&print_fn); +}, { bla => "r->in.bla" }, \&print_fn, "return;"); test_warnings("", sub { $fn->("r->in.bla"); }); -is($output, "if (r->in.bla == NULL) return NT_STATUS_INVALID_PARAMETER_MIX;"); +is($output, "if (r->in.bla == NULL) return;"); # Test case 2: Simple ref pointer dereference @@ -55,7 +55,7 @@ $fn = check_null_pointer({ }, ] } -}, { bla => "r->in.bla" }, \&print_fn); +}, { bla => "r->in.bla" }, \&print_fn, undef); test_warnings("", sub { $fn->("r->in.bla"); }); @@ -77,7 +77,7 @@ $fn = check_null_pointer({ }, ] } -}, { bla => "r->in.bla" }, \&print_fn); +}, { bla => "r->in.bla" }, \&print_fn, undef); test_warnings("nofile:1: too much dereferences for `bla'\n", sub { $fn->("r->in.bla"); }); @@ -104,12 +104,12 @@ $fn = check_null_pointer({ }, ] } -}, { bla => "r->in.bla" }, \&print_fn); +}, { bla => "r->in.bla" }, \&print_fn, "return;"); test_warnings("", sub { $fn->("*r->in.bla"); }); -is($output, "if (*r->in.bla == NULL) return NT_STATUS_INVALID_PARAMETER_MIX;"); +is($output, "if (*r->in.bla == NULL) return;"); # Test case 5: Unknown variable @@ -127,9 +127,9 @@ $fn = check_null_pointer({ }, ] } -}, { }, \&print_fn); +}, { }, \&print_fn, "return;"); 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;"); +is($output, "if (r->in.bla == NULL) return;"); -- cgit