| Subject: | Problem with ->compactref |
I encountered a problem trying to use ->compactref(\@list) where the (incorrect) results I was
receiving were closer to an expansion rather than a compaction -- the results were correct
when using Compact(@list) and ->compact(@list). I got consistent results with Perl 5.8.8,
5.10.1, and 5.12.3 under Linux and Darwin.
When I checked the code, it appeared that ->compactref() only took into account an array ref
of NetAddr::IP objects as its only argument, so Compact(@list) and ->compact(@list) worked
but ->compactref(\@list) did not since the first argument would be a NetAddr::IP object and
the second would be the array ref.
Here is a patch that should address that. I also implemented support and POD for
Compact(\@list), which made the most sense to me without having to add an export for a
"Compactref".
I've also included a patch to augment t/v4-compact.t
Thanks.
Rusty
| Subject: | compact-t.patch |
--- t/v4-compact.t 2008-12-02 12:54:12.000000000 -0800
+++ t/v4-compact-new.t 2011-05-17 12:35:14.000000000 -0700
@@ -18,44 +18,93 @@
exit 0;
}
-print "1..2\n";
+print "1..9\n";
-my @ips;
+my @ips1;
for my $ip ('10.0.0.0', '11.0.0.0', '12.0.0.0') {
- push @ips, NetAddr::IP->new($ip, 24)->split(32);
+ push @ips1, NetAddr::IP->new($ip, 24)->split(32);
}
for my $ip ('20.0.0.0', '30.0.0.0', '40.0.0.0') {
- push @ips, NetAddr::IP->new($ip, 16)->split(28);
+ push @ips1, NetAddr::IP->new($ip, 16)->split(28);
}
-my @c = Compact(@ips);
-my @m;
+my @ips2;
-for my $c (@c) {
- push @m, grep { $c->addr eq $_->[0] and $c->mask eq $_->[1] } @r;
+for my $num (0 .. 255) {
+ push @ips2, NetAddr::IP->new("192.168.$num.0", 24);
}
+my $ips2_compact = '192.168.0.0/16';
-if (@m == @c) {
- print "ok 1\n";
-}
-else {
- print "not ok 1\n";
-}
+# Compact(@)
+#
+compact_ips1_check(1, Compact(@ips1));
+compact_ips2_check(2, Compact(@ips2));
+
+# ->compact(@)
+#
+compact_ips1_check(3, $ips1[0]->compact(@ips1[1..$#ips1]));
+compact_ips2_check(4, $ips2[0]->compact(@ips2[1..$#ips2]));
+
+# Compact([])
+#
+compact_ips1_check(5, @{Compact(\@ips1)});
+compact_ips2_check(6, @{Compact(\@ips2)});
+
+# ->compactref([])
+#
+compact_ips1_check(7, @{$ips1[0]->compactref([@ips1[1..$#ips1]])});
+compact_ips2_check(8, @{$ips2[0]->compactref([@ips2[1..$#ips2]])});
-@ips = ();
+# duplicate IP
+#
+@ips1 = ();
for my $ip (qw(1.1.1.1 1.1.1.1 1.1.1.1 1.1.1.1)) {
- push(@ips, NetAddr::IP->new($ip));
+ push(@ips1, NetAddr::IP->new($ip));
}
-@c = NetAddr::IP::compact(@ips);
+@c = NetAddr::IP::compact(@ips1);
if (@c == 1 and $c[0]->cidr() eq '1.1.1.1/32') {
- print "ok 2\n";
+ print "ok 9\n";
}
else {
- print "not ok 2\n";
+ print "not ok 9\n";
}
+
+######################################################################
+sub compact_ips1_check
+{
+ my $num = shift;
+ my @ips = shift;
+
+ my @mips;
+ for my $ip (@ips) {
+ push @mips, grep { $ip->addr eq $_->[0] and $ip->mask eq $_->[1] } @r;
+ }
+
+ if (@mips == @ips) {
+ print "ok $num\n";
+ }
+ else {
+ print "not ok $num\n";
+ }
+}
+
+
+######################################################################
+sub compact_ips2_check
+{
+ my $num = shift;
+ my @ips = shift;
+
+ if (@ips == 1 and $ips[0] eq $ips2_compact) {
+ print "ok $num\n";
+ }
+ else {
+ print "not ok $num\n";
+ }
+}
| Subject: | compact.patch |
--- IP.pm 2011-04-06 11:31:14.000000000 -0700
+++ IP-new.pm 2011-05-17 12:34:21.000000000 -0700
@@ -390,7 +390,10 @@
}
sub compact {
- return @{compactref(\@_)};
+ return compactref($_[0])
+ if ref $_[0] eq 'ARRAY'; # Compact(\@list)
+
+ return @{compactref(\@_)}; # Compact(@list) or ->compact(@list)
}
*Compact = \&compact;
@@ -1084,7 +1087,9 @@
=item C<$me-E<gt>compactref(\@list)>
-As usual, a faster version of =item C<-E<gt>compact()> that returns a
+=item C<$compacted_object_list = Compact(\@list)>
+
+As usual, a faster version of C<-E<gt>compact()> that returns a
reference to a list. Note that this method takes a reference to a list
instead.
@@ -1097,39 +1102,57 @@
# or return [];
# return [] unless @r;
- return [] unless (my @unr = @{$_[0]});
+ my @r;
+ {
+ my $unr = [];
+ my $args = $_[0];
+
+ if (ref $_[0] eq __PACKAGE__ and ref $_[1] eq 'ARRAY') {
+ # ->compactref(\@list)
+ #
+ $unr = [$_[0], @{$_[1]}]; # keeping structures intact
+ }
+ else {
+ # Compact(@list) or ->compact(@list) or Compact(\@list)
+ #
+ $unr = $args;
+ }
+
+ return [] unless @$unr;
- foreach(0..$#unr) {
- $unr[$_]->{addr} = $unr[$_]->network->{addr};
+ for (@$unr) {
+ $_->{addr} = $_->network->{addr};
+ }
+
+ @r = sort @$unr;
}
- my @r = sort @unr;
my $changed;
do {
- $changed = 0;
- for(my $i=0; $i <= $#r -1;$i++) {
- if ($r[$i]->contains($r[$i +1])) {
- splice(@r,$i +1,1);
- ++$changed;
- --$i;
- }
- elsif ((notcontiguous($r[$i]->{mask}))[1] == (notcontiguous($r[$i +1]->{mask}))[1]) { # masks the same
- if (hasbits($r[$i]->network->{addr} ^ $r[$i +1]->network->{addr})) { # if not the same netblock
- my $upnet = $r[$i]->copy;
- $upnet->{mask} = shiftleft($upnet->{mask},1);
- if ($upnet->contains($r[$i +1])) { # adjacent nets in next net up
- $r[$i] = $upnet;
- splice(@r,$i +1,1);
- ++$changed;
- --$i;
- }
- } else { # identical nets
- splice(@r,$i +1,1);
- ++$changed;
- --$i;
- }
- }
- }
+ $changed = 0;
+ for(my $i=0; $i <= $#r -1;$i++) {
+ if ($r[$i]->contains($r[$i +1])) {
+ splice(@r,$i +1,1);
+ ++$changed;
+ --$i;
+ }
+ elsif ((notcontiguous($r[$i]->{mask}))[1] == (notcontiguous($r[$i +1]->{mask}))[1]) { # masks the same
+ if (hasbits($r[$i]->{addr} ^ $r[$i +1]->{addr})) { # if not the same netblock
+ my $upnet = $r[$i]->copy;
+ $upnet->{mask} = shiftleft($upnet->{mask},1);
+ if ($upnet->contains($r[$i +1])) { # adjacent nets in next net up
+ $r[$i] = $upnet;
+ splice(@r,$i +1,1);
+ ++$changed;
+ --$i;
+ }
+ } else { # identical nets
+ splice(@r,$i +1,1);
+ ++$changed;
+ --$i;
+ }
+ }
+ }
} while $changed;
return \@r;
}