From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) by lore.proxmox.com (Postfix) with ESMTPS id 3563A1FF16B for ; Thu, 31 Oct 2024 18:07:55 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 1D203DF87; Thu, 31 Oct 2024 18:08:01 +0100 (CET) From: Max Carrara To: pve-devel@lists.proxmox.com Date: Thu, 31 Oct 2024 18:07:18 +0100 Message-Id: <20241031170720.338794-4-m.carrara@proxmox.com> X-Mailer: git-send-email 2.39.5 In-Reply-To: <20241031170720.338794-1-m.carrara@proxmox.com> References: <20241031170720.338794-1-m.carrara@proxmox.com> MIME-Version: 1.0 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.034 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 v3 pve-common 3/5] 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 v2 --> v3: * none 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 77d0c17..05cd538 100644 --- a/src/PVE/SectionConfig.pm +++ b/src/PVE/SectionConfig.pm @@ -1177,25 +1177,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) = @_; @@ -1206,105 +1207,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.5 _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel