From 1e078b8c8d40bb59434d9a865b5e5ce7a4227734 Mon Sep 17 00:00:00 2001
From: Tom Eastep <teastep@shorewall.net>
Date: Fri, 9 Apr 2010 09:38:03 -0700
Subject: [PATCH] Use splice() to delete rules from chains

Signed-off-by: Tom Eastep <teastep@shorewall.net>
---
 Shorewall/Perl/Shorewall/Chains.pm | 82 +++++++++++++++---------------
 Shorewall/releasenotes.txt         |  7 ++-
 2 files changed, 45 insertions(+), 44 deletions(-)

diff --git a/Shorewall/Perl/Shorewall/Chains.pm b/Shorewall/Perl/Shorewall/Chains.pm
index d2f7f4aa1..4dea4071c 100644
--- a/Shorewall/Perl/Shorewall/Chains.pm
+++ b/Shorewall/Perl/Shorewall/Chains.pm
@@ -593,23 +593,6 @@ sub add_reference ( $$ ) {
     $toref->{references}{$fromref->{name}}++;
 }
 
-#
-# Compress out undefined elements in rules - beginning with 4.4.9, this gets called any time that
-# a rule other than the last one in the chain is deleted (set to 'undef').
-#
-sub compress_rules( $ ) {
-    my $chainref = shift;
-    my @rules;
-
-    for ( @{$chainref->{rules}} ) {
-	push @rules, $_ if defined;
-    }
-
-    $chainref->{rules} = \@rules;
-
-    trace( $chainref, 'C', undef, '' ) if $debug;
-}  
-
 #
 # Purge jumps previously added via add_jump. If the target chain is empty, reset its
 # referenced flag
@@ -617,21 +600,31 @@ sub compress_rules( $ ) {
 sub purge_jump ( $$ ) {
     my ( $fromref, $toref ) = @_;
     my $to = $toref->{name};
-    my $rule = 0;
-    my $rules = @{$fromref->{rules}};
-    my $deleted = 0;
+    #
+    # splice() of an array being iterated over causes elements to be skipped so
+    # we need to restart the scan after each deletion.
+    #
+    my $progress = 1;
 
-    for ( @{$fromref->{rules}} ) {
-	$rule++;
-	if ( / -[gj] ${to}\b/ ) {
-	    trace( $fromref, 'D', $rule, $_ ) if $debug;
-	    $_ = undef;
-	    $deleted = 1 unless $rule == $rules && $rules > 1;
+    while ( $progress ) {
+	my $rule = 0;
+	
+	$progress = 0;
+
+	for ( @{$fromref->{rules}} ) {
+	    if ( / -[gj] ${to}\b/ ) {
+		trace( $fromref, 'D', $rule + 1, $_ ) if $debug;
+		splice(  @{$fromref->{rules}}, $rule, 1 );
+		$progress = 1;
+		last;
+	    }
+		
+	    $rule++;
 	}
     }
 
-    compress_rules( $fromref ) if $deleted;
-
+    delete $toref->{references}{$fromref->{name}};
+    
     unless ( @{$toref->{rules}} ) {
 	$toref->{referenced} = 0;
 	trace ( $toref, 'X', undef, '' ) if $debug;
@@ -1438,22 +1431,31 @@ sub delete_references( $ ) {
     my $count    = 0;
     
     for my $fromref ( map $chain_table{$table}{$_} , keys %{$chainref->{references}} ) {
-	my $rule = 0;
-	my $deleted = 0;
-	my $rules = @{$fromref->{rules}};
+	#
+	# splice() of an array being iterated over causes elements to be skipped so
+	# we need to restart the scan after each deletion.
+	#
+	my $progress = 1;
+	
+	while ( $progress ) {
+	    my $rule = 0;
 
-	for ( @{$fromref->{rules}} ) {
-	    $rule++;
+	    $progress = 0;
 
-	    if ( / -[jg] $chainref->{name}$/ ) {
-		trace( $fromref, 'D', $rule, $_ ) if $debug;
-		$_ = undef;
-		$count++;
-		$deleted = 1 unless $rule == $rules && $rules > 1;
+	    for ( @{$fromref->{rules}} ) {
+		if ( / -[jg] $chainref->{name}$/ ) {
+		    trace( $fromref, 'D', $rule + 1, $_ ) if $debug;
+		    splice( @{$fromref->{rules}}, $rule, 1 );
+		    $count++;
+		    $progress = 1;
+		    last;
+		}
+		
+		$rule++;
 	    }
 	}
-	    
-	compress_rules( $fromref ) if $deleted;
+
+	delete $chainref->{references}{$fromref->{name}};
     }
 
     if ( $count ) {
diff --git a/Shorewall/releasenotes.txt b/Shorewall/releasenotes.txt
index 2b1c17f2f..706481c6e 100644
--- a/Shorewall/releasenotes.txt
+++ b/Shorewall/releasenotes.txt
@@ -293,9 +293,8 @@ None.
 	I - Inserted a rule into a chain.
 	T - Shell source text appended/inserted into a chain --
             converted into rules at run-time.
-	D - Deleted Rule from a chain
-	C - Compressed the rules array for a chain to remove deleted
-   	    rules. This renumbers the remaining rules.
+	D - Deleted Rule from a chain; note that this causes the 
+	    following rules to be renumbered.
 	X - Deleted a chain
 
     Netfilter trace records indicate the table and chain being
@@ -304,7 +303,7 @@ None.
 
     Example (append the first rule to the filter FORWARD chain):
 
-	NF-(A)-> filter:FORWARD:1 
+	NF-(A)-> filter:FORWARD:1 ...
 
     If the trace record involves the chain itself, then no rule number
     is present.