Re: [PATCH 1/2] Improve indentation and HTML output of PrintActualLeases & leasesort

[Leo Hofmann <hofmann@leo-andres.de>]

[-- Attachment #1: Type: text/plain, Size: 8257 bytes --]

Hi Michael,
thank you for reviewing & merging my patch!

I agree, this was an interesting read. I think the web interface code could be improved. Feel free to point me to other issues, maybe I can contribute more.

Best regards,
Leo

Am 23.10.2020 um 12:07 schrieb Michael Tremer:
> Hey Leo-Andres,
>
> Thank you for splitting the patch. This was easier to review.
>
> It is still interesting code and it is very interesting too that all the DHCP stuff is in header.pl.
>
> But that is outside the scope of this patch.
>
> I will merge it. Thank you!
>
> Best,
> -Michael
>
>> On 21 Oct 2020, at 11:48, Leo-Andres Hofmann <hofmann(a)leo-andres.de> wrote:
>>
>> Signed-off-by: Leo-Andres Hofmann <hofmann(a)leo-andres.de>
>> ---
>> config/cfgroot/header.pl | 159 +++++++++++++++++++--------------------
>> 1 file changed, 79 insertions(+), 80 deletions(-)
>>
>> diff --git a/config/cfgroot/header.pl b/config/cfgroot/header.pl
>> index 1046f5992..6322cfebe 100644
>> --- a/config/cfgroot/header.pl
>> +++ b/config/cfgroot/header.pl
>> @@ -403,66 +403,65 @@ sub PrintActualLeases
>> <th width='25%' align='center'><a href='$ENV{'SCRIPT_NAME'}?ETHER'><b>$tr{'mac address'}</b></a></th>
>> <th width='20%' align='center'><a href='$ENV{'SCRIPT_NAME'}?HOSTNAME'><b>$tr{'hostname'}</b></a></th>
>> <th width='25%' align='center'><a href='$ENV{'SCRIPT_NAME'}?ENDTIME'><b>$tr{'lease expires'} (local time d/m/y)</b></a></th>
>> -<th width='5%' align='center'><b>Add to fix leases<b></th>
>> +<th width='5%' align='center'><b>Add to fix leases</b></th>
>> </tr>
>> END
>> ;
>>
>>      open(LEASES,"/var/state/dhcp/dhcpd.leases") or die "Can't open dhcpd.leases";
>> -    while ($line = <LEASES>) {
>> -	next if( $line =~ /^\s*#/ );
>> -	chomp($line);
>> -	@temp = split (' ', $line);
>> -
>> -	if ($line =~ /^\s*lease/) {
>> -	    $ip = $temp[1];
>> -	    #All field are not necessarily read. Clear everything
>> -	    $endtime = 0;
>> -	    $ether = "";
>> -	    $hostname = "";
>> -	}
>> +	while (my $line = <LEASES>) {
>> +		next if( $line =~ /^\s*#/ );
>> +		chomp($line);
>> +		@temp = split (' ', $line);
>> +
>> +		if ($line =~ /^\s*lease/) {
>> +			$ip = $temp[1];
>> +			#All field are not necessarily read. Clear everything
>> +			$endtime = 0;
>> +			$ether = "";
>> +			$hostname = "";
>> +		}
>>
>> -	if ($line =~ /^\s*ends/) {
>> -	    $line =~ /(\d+)\/(\d+)\/(\d+) (\d+):(\d+):(\d+)/;
>> -	    $endtime = timegm($6, $5, $4, $3, $2 - 1, $1 - 1900);
>> -	}
>> +		if ($line =~ /^\s*ends/) {
>> +			$line =~ /(\d+)\/(\d+)\/(\d+) (\d+):(\d+):(\d+)/;
>> +			$endtime = timegm($6, $5, $4, $3, $2 - 1, $1 - 1900);
>> +		}
>>
>> -	if ($line =~ /^\s*hardware ethernet/) {
>> -	    $ether = $temp[2];
>> -	    $ether =~ s/;//g;
>> -	}
>> +		if ($line =~ /^\s*hardware ethernet/) {
>> +			$ether = $temp[2];
>> +			$ether =~ s/;//g;
>> +		}
>>
>> -	if ($line =~ /^\s*client-hostname/) {
>> -	    $hostname = "$temp[1] $temp[2] $temp[3]";
>> -	    $hostname =~ s/;//g;
>> -	    $hostname =~ s/\"//g;
>> -	}
>> +		if ($line =~ /^\s*client-hostname/) {
>> +			$hostname = "$temp[1] $temp[2] $temp[3]";
>> +			$hostname =~ s/;//g;
>> +			$hostname =~ s/\"//g;
>> +		}
>>
>> -	if ($line eq "}") {
>> -	    @record = ('IPADDR',$ip,'ENDTIME',$endtime,'ETHER',$ether,'HOSTNAME',$hostname);
>> -    	    $record = {};                        		# create a reference to empty hash
>> -	    %{$record} = @record;                		# populate that hash with @record
>> -	    $entries{$record->{'IPADDR'}} = $record;   	# add this to a hash of hashes
>> -	}
>> +		if ($line eq "}") {
>> +			@record = ('IPADDR',$ip,'ENDTIME',$endtime,'ETHER',$ether,'HOSTNAME',$hostname);
>> +			$record = {};								# create a reference to empty hash
>> +			%{$record} = @record;						# populate that hash with @record
>> +			$entries{$record->{'IPADDR'}} = $record;	# add this to a hash of hashes
>> +		}
>>      }
>>      close(LEASES);
>>
>>      my $id = 0;
>> -    my $col="";
>> +    my $col = "";
>>      foreach my $key (sort leasesort keys %entries) {
>> -	print "<form method='post' action='/cgi-bin/dhcp.cgi'>\n";
>> -	my $hostname = &cleanhtml($entries{$key}->{HOSTNAME},"y");
>> +		print "<form method='post' action='/cgi-bin/dhcp.cgi'>\n";
>> +		my $hostname = &cleanhtml($entries{$key}->{HOSTNAME},"y");
>>
>> -	if ($id % 2) {
>> -	    print "<tr>";
>> -	    $col="bgcolor='$table1colour'";
>> -	}
>> -	else {
>> -	    print "<tr>";
>> -	    $col="bgcolor='$table2colour'";
>> -	}
>> +		if ($id % 2) {
>> +			print "<tr>";
>> +			$col="bgcolor='$table1colour'";
>> +		} else {
>> +			print "<tr>";
>> +			$col="bgcolor='$table2colour'";
>> +		}
>>
>> -	print <<END
>> +		print <<END
>> <td align='center' $col><input type='hidden' name='FIX_ADDR' value='$entries{$key}->{IPADDR}' />$entries{$key}->{IPADDR}</td>
>> <td align='center' $col><input type='hidden' name='FIX_MAC' value='$entries{$key}->{ETHER}' />$entries{$key}->{ETHER}</td>
>> <td align='center' $col><input type='hidden' name='FIX_REMARK' value='$hostname' />&nbsp;$hostname</td>
>> @@ -470,20 +469,20 @@ END
>> END
>> ;
>>
>> -	($sec, $min, $hour, $mday, $mon, $year, $wday, $yday, $dst) = localtime ($entries{$key}->{ENDTIME});
>> -	$enddate = sprintf ("%02d/%02d/%d %02d:%02d:%02d",$mday,$mon+1,$year+1900,$hour,$min,$sec);
>> +		($sec, $min, $hour, $mday, $mon, $year, $wday, $yday, $dst) = localtime ($entries{$key}->{ENDTIME});
>> +		$enddate = sprintf ("%02d/%02d/%d %02d:%02d:%02d",$mday,$mon+1,$year+1900,$hour,$min,$sec);
>>
>> -	if ($entries{$key}->{ENDTIME} < time() ){
>> -	    print "<strike>$enddate</strike>";
>> -	} else {
>> -	    print "$enddate";
>> -	}
>> -	print <<END
>> +		if ($entries{$key}->{ENDTIME} < time() ){
>> +			print "<strike>$enddate</strike>";
>> +		} else {
>> +			print "$enddate";
>> +		}
>> +		print <<END
>> </td><td $col><input type='hidden' name='ACTION' value='$Lang::tr{'add'}2' /><input type='submit' name='SUBMIT' value='$Lang::tr{'add'}' />
>> </td></tr></form>
>> END
>> ;
>> -	$id++;
>> +		$id++;
>>      }
>>
>>      print "</table>";
>> @@ -493,34 +492,34 @@ END
>>
>> # This sub is used during display of actives leases
>> sub leasesort {
>> -    if (rindex ($dhcpsettings{'SORT_LEASELIST'},'Rev') != -1)
>> -    {
>> -        $qs=substr ($dhcpsettings{'SORT_LEASELIST'},0,length($dhcpsettings{'SORT_LEASELIST'})-3);
>> -        if ($qs eq 'IPADDR') {
>> -            @a = split(/\./,$entries{$a}->{$qs});
>> -            @b = split(/\./,$entries{$b}->{$qs});
>> -            ($b[0]<=>$a[0]) ||
>> -            ($b[1]<=>$a[1]) ||
>> -            ($b[2]<=>$a[2]) ||
>> -            ($b[3]<=>$a[3]);
>> -        }else {
>> -            $entries{$b}->{$qs} cmp $entries{$a}->{$qs};
>> -        }
>> -    }
>> -    else #not reverse
>> -    {
>> -        $qs=$dhcpsettings{'SORT_LEASELIST'};
>> -        if ($qs eq 'IPADDR') {
>> -	    @a = split(/\./,$entries{$a}->{$qs});
>> -    	    @b = split(/\./,$entries{$b}->{$qs});
>> -    	    ($a[0]<=>$b[0]) ||
>> -	    ($a[1]<=>$b[1]) ||
>> -	    ($a[2]<=>$b[2]) ||
>> -    	    ($a[3]<=>$b[3]);
>> -	}else {
>> -    	    $entries{$a}->{$qs} cmp $entries{$b}->{$qs};
>> +	if (rindex ($dhcpsettings{'SORT_LEASELIST'},'Rev') != -1)
>> +	{
>> +		$qs=substr ($dhcpsettings{'SORT_LEASELIST'},0,length($dhcpsettings{'SORT_LEASELIST'})-3);
>> +		if ($qs eq 'IPADDR') {
>> +			@a = split(/\./,$entries{$a}->{$qs});
>> +			@b = split(/\./,$entries{$b}->{$qs});
>> +			($b[0]<=>$a[0]) ||
>> +			($b[1]<=>$a[1]) ||
>> +			($b[2]<=>$a[2]) ||
>> +			($b[3]<=>$a[3]);
>> +		} else {
>> +			$entries{$b}->{$qs} cmp $entries{$a}->{$qs};
>> +		}
>> +	}
>> +	else #not reverse
>> +	{
>> +		$qs=$dhcpsettings{'SORT_LEASELIST'};
>> +		if ($qs eq 'IPADDR') {
>> +			@a = split(/\./,$entries{$a}->{$qs});
>> +			@b = split(/\./,$entries{$b}->{$qs});
>> +			($a[0]<=>$b[0]) ||
>> +			($a[1]<=>$b[1]) ||
>> +			($a[2]<=>$b[2]) ||
>> +			($a[3]<=>$b[3]);
>> +		} else {
>> +			$entries{$a}->{$qs} cmp $entries{$b}->{$qs};
>> +		}
>> 	}
>> -    }
>> }
>>
>> sub colorize {
>> -- 
>> 2.27.0.windows.1
>>
>>