From mboxrd@z Thu Jan  1 00:00:00 1970
From: Leo-Andres Hofmann <hofmann@leo-andres.de>
To: development@lists.ipfire.org
Subject: [PATCH v2 1/6] zoneconf.cgi: Change NIC display order, improve code
Date: Thu, 18 Feb 2021 15:30:11 +0100
Message-ID: <20210218143016.972-1-hofmann@leo-andres.de>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="===============1619821465915014528=="
List-Id: <development.lists.ipfire.org>

--===============1619821465915014528==
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: quoted-printable

Refactor duplicate perl code and add comments

Signed-off-by: Leo-Andres Hofmann <hofmann(a)leo-andres.de>
---
 html/cgi-bin/zoneconf.cgi | 53 +++++++++++++++++++++++++--------------
 1 file changed, 34 insertions(+), 19 deletions(-)

diff --git a/html/cgi-bin/zoneconf.cgi b/html/cgi-bin/zoneconf.cgi
index 0914ceb78..bf46ab0c7 100644
--- a/html/cgi-bin/zoneconf.cgi
+++ b/html/cgi-bin/zoneconf.cgi
@@ -26,6 +26,7 @@ require '/var/ipfire/general-functions.pl';
 require "${General::swroot}/lang.pl";
 require "${General::swroot}/header.pl";
=20
+###--- HTML HEAD ---###
 my $extraHead =3D <<END
 <style>
 	table#zoneconf {
@@ -105,7 +106,9 @@ my $extraHead =3D <<END
 <script src=3D"/include/zoneconf.js"></script>
 END
 ;
+###--- END HTML HEAD ---###
=20
+### Read configuration ###
 my %ethsettings =3D ();
 my %vlansettings =3D ();
 my %cgiparams =3D ();
@@ -119,7 +122,7 @@ my $restart_notice =3D "";
 &Header::showhttpheaders();
=20
 # Define all zones we will check for NIC assignment
-my @zones =3D ("green", "red", "orange", "blue");
+my @zones =3D ("red", "green", "orange", "blue");
=20
 # Get all physical NICs present
 opendir(my $dh, "/sys/class/net/");
@@ -153,6 +156,21 @@ foreach (@nics) {
 	}
 }
=20
+### Functions ###
+
+# Check if a zone is in IP mode or in PPP, PPPoE, VDSL, ... mode
+sub is_zonetype_ip {
+	my $zone_type =3D shift;
+	return ($zone_type eq "STATIC" || $zone_type eq "DHCP");
+}
+
+# Check if a zone is activated (device assigned)
+sub is_zone_activated {
+	my $zone =3D uc shift;
+	return ($ethsettings{"${zone}_DEV"} ne "");
+}
+
+### START PAGE ###
 &Header::openpage($Lang::tr{"zoneconf title"}, 1, $extraHead);
 &Header::openbigbox('100%', 'center');
=20
@@ -195,6 +213,7 @@ if ($cgiparams{"ACTION"} eq $Lang::tr{"save"}) {
 				}
 			}
=20
+			# skip NIC/VLAN assignment and additional zone options for RED in PPP mode
 			next;
 		}
=20
@@ -278,6 +297,7 @@ if ($cgiparams{"ACTION"} eq $Lang::tr{"save"}) {
 		}
 	}
=20
+	# validation failed, show error message and exit
 	if ($VALIDATE_error) {
 		&Header::openbox('100%', 'left', $Lang::tr{"error"});
=20
@@ -290,16 +310,17 @@ if ($cgiparams{"ACTION"} eq $Lang::tr{"save"}) {
 		exit 0;
 	}
=20
+	# new settings are valid, write configuration files
 	&General::writehash("${General::swroot}/ethernet/settings",\%ethsettings);
 	&General::writehash("${General::swroot}/ethernet/vlans",\%vlansettings);
=20
 	$restart_notice =3D $Lang::tr{'zoneconf notice reboot'};
 }
=20
-&Header::openbox('100%', 'left', $Lang::tr{"zoneconf nic assignment"});
-
 ### START OF TABLE ###
=20
+&Header::openbox('100%', 'left', $Lang::tr{"zoneconf nic assignment"});
+
 print <<END
 <form method=3D'post' enctype=3D'multipart/form-data'>
 	<table id=3D"zoneconf">
@@ -311,19 +332,16 @@ END
 # Fill the table header with all activated zones
 foreach (@zones) {
 	my $uc =3D uc $_;
-	my $dev_name =3D $ethsettings{"${uc}_DEV"};
=20
-	if ($dev_name eq "") { # If the zone is not activated, don't show it
-		next;
-	}
+	# If the zone is not activated, don't show it
+	next unless is_zone_activated($_);
=20
-	# If the zone is in PPP mode, don't show a mode dropdown
+	# If the red zone is in PPP mode, don't show a mode dropdown
 	if ($uc eq "RED") {
 		my $red_type =3D $ethsettings{"RED_TYPE"};
-		my $red_restricted =3D ($uc eq "RED" && ! ($red_type eq "STATIC" || $red_t=
ype eq "DHCP"));
=20
-		if ($red_restricted) {
-			print "\t\t<td class=3D'heading $_'>$uc ($red_type)</td>\n";
+		unless (is_zonetype_ip($red_type)) {
+			print "\t\t<td class=3D'heading bold $_'>$uc ($red_type)</td>\n";
=20
 			next; # We're done here
 		}
@@ -354,6 +372,7 @@ END
=20
 print "\t</tr>\n";
=20
+# NIC assignment matrix
 foreach (@nics) {
 	my $mac =3D $_->[0];
 	my $nic =3D $_->[1];
@@ -365,19 +384,14 @@ foreach (@nics) {
 	# Iterate through all zones and check if the current NIC is assigned to it
 	foreach (@zones) {
 		my $uc =3D uc $_;
-		my $dev_name =3D $ethsettings{"${uc}_DEV"};
 		my $highlight =3D "";
=20
-		if ($dev_name eq "") { # Again, skip the zone if it is not activated
-			next;
-		}
+		# If the zone is not activated, don't show it
+		next unless is_zone_activated($_);
=20
 		if ($uc eq "RED") {
-			my $red_type =3D $ethsettings{"RED_TYPE"};
-			my $red_restricted =3D ($uc eq "RED" && ! ($red_type eq "STATIC" || $red_=
type eq "DHCP"));
-
 			# VLANs/Bridging is not possible if the RED interface is set to PPP, PPPo=
E, VDSL, ...
-			if ($red_restricted) {
+			unless (is_zonetype_ip($ethsettings{"RED_TYPE"})) {
 				my $checked =3D "";
=20
 				if ($mac eq $ethsettings{"${uc}_MACADDR"}) {
@@ -449,6 +463,7 @@ END
 	print "\t</tr>\n";
 }
=20
+# footer and submit button
 print <<END
 	</table>
=20
--=20
2.27.0.windows.1


--===============1619821465915014528==--