From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9]) by lore.proxmox.com (Postfix) with ESMTPS id 6C7B61FF2A8 for ; Tue, 2 Jul 2024 16:13:32 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id AFC6F1F996; Tue, 2 Jul 2024 16:13:47 +0200 (CEST) From: Max Carrara To: pve-devel@lists.proxmox.com Date: Tue, 2 Jul 2024 16:13:13 +0200 Message-Id: <20240702141314.445130-4-m.carrara@proxmox.com> X-Mailer: git-send-email 2.39.2 In-Reply-To: <20240702141314.445130-1-m.carrara@proxmox.com> References: <20240702141314.445130-1-m.carrara@proxmox.com> MIME-Version: 1.0 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.030 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: [pve-devel] [PATCH v2 pve-common 3/4] section config: clean up parser logic and semantics X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Proxmox VE development discussion Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pve-devel-bounces@lists.proxmox.com Sender: "pve-devel" In order to make the parser somewhat more maintainable in the future, this commit cleans up its logic and makes its control flow easier to follow. Furthermore, regexes are assigned to variables and some variables are renamed in order to make the code more semantically expressive. Signed-off-by: Max Carrara --- Changes v1 --> v2: * reword commit message * for errors, use reference of array instead of array directly * drop 'x' modifier from $re_kv_pair regex, as it doesn't have any benefit src/PVE/SectionConfig.pm | 187 ++++++++++++++++++++------------------- 1 file changed, 97 insertions(+), 90 deletions(-) diff --git a/src/PVE/SectionConfig.pm b/src/PVE/SectionConfig.pm index a6cc468..ec074fa 100644 --- a/src/PVE/SectionConfig.pm +++ b/src/PVE/SectionConfig.pm @@ -1190,25 +1190,26 @@ The error. sub parse_config { my ($class, $filename, $raw, $allow_unknown) = @_; - my $pdata = $class->private(); + if (!defined($raw)) { + return { + ids => {}, + order => {}, + digest => Digest::SHA::sha1_hex(''), + }; + } + + my $re_begins_with_comment = qr/^\s*#/; + my $re_kv_pair = qr/^\s+(\S+)(\s+(.*\S))?\s*$/; my $ids = {}; my $order = {}; - - $raw = '' if !defined($raw); - my $digest = Digest::SHA::sha1_hex($raw); - my $pri = 1; + my $current_section_no = 1; + my $line_no = 0; - my $lineno = 0; my @lines = split(/\n/, $raw); - my $nextline = sub { - while (defined(my $line = shift @lines)) { - $lineno++; - return $line if ($line !~ /^\s*#/); - } - }; + my $errors = []; my $is_array = sub { my ($type, $key) = @_; @@ -1219,105 +1220,111 @@ sub parse_config { return $schema->{type} eq 'array'; }; - my $errors = []; - while (@lines) { - my $line = $nextline->(); + my $get_next_line = sub { + while (scalar(@lines)) { + my $line = shift(@lines); + $line_no++; + + next if ($line =~ m/$re_begins_with_comment/); + + return $line; + } + + return undef; + }; + + my $skip_to_next_empty_line = sub { + while ($get_next_line->() ne '') {} + }; + + while (defined(my $line = $get_next_line->())) { next if !$line; - my $errprefix = "file $filename line $lineno"; + my $errprefix = "file $filename line $line_no"; - my ($type, $sectionId, $errmsg, $config) = $class->parse_section_header($line); - if ($config) { - my $skip = 0; - my $unknown = 0; + my ($type, $section_id, $errmsg, $config) = $class->parse_section_header($line); - my $plugin; + if (!defined($config)) { + warn "$errprefix - ignore config line: $line\n"; + next; + } - if ($errmsg) { - $skip = 1; - chomp $errmsg; - warn "$errprefix (skip section '$sectionId'): $errmsg\n"; - } elsif (!$type) { - $skip = 1; - warn "$errprefix (skip section '$sectionId'): missing type - internal error\n"; - } else { - if (!($plugin = $pdata->{plugins}->{$type})) { - if ($allow_unknown) { - $unknown = 1; - } else { - $skip = 1; - warn "$errprefix (skip section '$sectionId'): unsupported type '$type'\n"; - } - } - } + if ($errmsg) { + chomp $errmsg; + warn "$errprefix (skip section '$section_id'): $errmsg\n"; + $skip_to_next_empty_line->(); + next; + } + + if (!$type) { + warn "$errprefix (skip section '$section_id'): missing type - internal error\n"; + $skip_to_next_empty_line->(); + next; + } + + my $plugin = eval { $class->lookup($type) }; + my $is_unknown_type = defined($@) && $@ ne ''; + + if ($is_unknown_type && !$allow_unknown) { + warn "$errprefix (skip section '$section_id'): unsupported type '$type'\n"; + $skip_to_next_empty_line->(); + next; + } - while ($line = $nextline->()) { - next if $skip; - - $errprefix = "file $filename line $lineno"; - - if ($line =~ m/^\s+(\S+)(\s+(.*\S))?\s*$/) { - my ($k, $v) = ($1, $3); - - eval { - if ($unknown) { - if (!defined($config->{$k})) { - $config->{$k} = $v; - } else { - if (!ref($config->{$k})) { - $config->{$k} = [$config->{$k}]; - } - push $config->{$k}->@*, $v; - } - } elsif ($is_array->($type, $k)) { - $v = $plugin->check_value($type, $k, $v, $sectionId); - $config->{$k} = [] if !defined($config->{$k}); - push $config->{$k}->@*, $v; + # Parse kv-pairs of section - will go on until empty line is encountered + while (my $section_line = $get_next_line->()) { + if ($section_line =~ m/$re_kv_pair/) { + my ($key, $value) = ($1, $3); + + eval { + if ($is_unknown_type) { + if (!defined($config->{$key})) { + $config->{$key} = $value; } else { - die "duplicate attribute\n" if defined($config->{$k}); - $v = $plugin->check_value($type, $k, $v, $sectionId); - $config->{$k} = $v; + $config->{$key} = [$config->{$key}] if !ref($config->{$key}); + push $config->{$key}->@*, $value; } - }; - if (my $err = $@) { - warn "$errprefix (section '$sectionId') - unable to parse value of '$k': $err"; - push $errors->@*, { - context => $errprefix, - section => $sectionId, - key => $k, - err => $err, - }; + } elsif ($is_array->($type, $key)) { + $value = $plugin->check_value($type, $key, $value, $section_id); + $config->{$key} = [] if !defined($config->{$key}); + push $config->{$key}->@*, $value; + } else { + die "duplicate attribute\n" if defined($config->{$key}); + $value = $plugin->check_value($type, $key, $value, $section_id); + $config->{$key} = $value; } - - } else { - warn "$errprefix (section '$sectionId') - ignore config line: $line\n"; + }; + if (my $err = $@) { + warn "$errprefix (section '$section_id') - unable to parse value of '$key': $err"; + push $errors->@*, { + context => $errprefix, + section => $section_id, + key => $key, + err => $err, + }; } } + } - if ($unknown) { - $config->{type} = $type; - $ids->{$sectionId} = $config; - $order->{$sectionId} = $pri++; - } elsif (!$skip && $type && $plugin && $config) { - $config->{type} = $type; - if (!$unknown) { - $config = eval { $config = $plugin->check_config($sectionId, $config, 1, 1); }; - warn "$errprefix (skip section '$sectionId'): $@" if $@; - } - $ids->{$sectionId} = $config; - $order->{$sectionId} = $pri++; + if ($is_unknown_type || ($type && $plugin && $config)) { + $config->{type} = $type; + + if (!$is_unknown_type) { + $config = eval { $config = $plugin->check_config($section_id, $config, 1, 1); }; + warn "$errprefix (skip section '$section_id'): $@" if $@; } - } else { - warn "$errprefix - ignore config line: $line\n"; + $ids->{$section_id} = $config; + $order->{$section_id} = $current_section_no++; } } my $cfg = { ids => $ids, order => $order, - digest => $digest + digest => $digest, }; + $cfg->{errors} = $errors if scalar($errors->@*) > 0; return $cfg; -- 2.39.2 _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel