[pve-devel] [PATCH common v4 2/7] fix #3893: network: add vlan id and range parameter definitions
Fiona Ebner
f.ebner at proxmox.com
Tue Sep 10 11:21:36 CEST 2024
Am 29.07.24 um 13:55 schrieb Aaron Lauterer:
> @@ -595,6 +601,34 @@ sub pve_verify_iface {
> return $id;
> }
>
> +# vlan id (vids), single or range
> +register_format('pve-vlan-id-or-range', \&pve_verify_vlan_id_or_range);
> +sub pve_verify_vlan_id_or_range {
> + my ($vlan, $noerr) = @_;
> +
> + my $check_vid = sub {
> + my $tag = shift;
> + if ( $tag < 2 || $tag > 4094) {
Style nit: extra space after parenthesis
> + return undef if $noerr;
> + die "invalid VLAN tag '$tag'\n";
> + }
> + };
> +
> + if ($vlan !~ m/^(\d+)(?:-(\d+))?$/) {
> + return undef if $noerr;
> + die "invalid VLAN configuration '$vlan'\n";
> + }
> + my $start = $1;
> + my $end = $2;
> + return undef if (!defined $check_vid->($start));
check_vid does not explicitly return anything in the success case, so
this check seems very brittle and I guess relies on implicit return?
Style nits:
parentheses for post-if
missing parentheses for defined()
perlcritic prefers 'return' over 'return undef'
> + if ($end) {
> + return undef if (!defined $check_vid->($end));
> + die "VLAN range must go from lower to higher tag '$vlan'" if $start >= $end && !$noerr;
What if $noerr is set, but $start >= $end? You want to have a 'return'
then, right?
> + }
> +
> + return $vlan;
> +}
> +
> # general addresses by name or IP
> register_format('address', \&pve_verify_address);
> sub pve_verify_address {
More information about the pve-devel
mailing list