From: Max Carrara <m.carrara@proxmox.com>
To: pve-devel@lists.proxmox.com
Subject: [pve-devel] [PATCH v1 pve-common 3/3] section config: clean up parser logic
Date: Tue, 4 Jun 2024 11:28:50 +0200 [thread overview]
Message-ID: <20240604092850.126083-4-m.carrara@proxmox.com> (raw)
In-Reply-To: <20240604092850.126083-1-m.carrara@proxmox.com>
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.
Signed-off-by: Max Carrara <m.carrara@proxmox.com>
---
src/PVE/SectionConfig.pm | 189 ++++++++++++++++++++-------------------
1 file changed, 98 insertions(+), 91 deletions(-)
diff --git a/src/PVE/SectionConfig.pm b/src/PVE/SectionConfig.pm
index a6b0183..30faaa4 100644
--- a/src/PVE/SectionConfig.pm
+++ b/src/PVE/SectionConfig.pm
@@ -1014,25 +1014,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*$/x;
my $ids = {};
my $order = {};
-
- $raw = '' if !defined($raw);
-
my $digest = Digest::SHA::sha1_hex($raw);
- my $pri = 1;
+ my $current_order = 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) = @_;
@@ -1043,106 +1044,112 @@ 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;
+ }
+
+ # 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);
- 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;
+ 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_order++;
}
}
my $cfg = {
ids => $ids,
order => $order,
- digest => $digest
+ digest => $digest,
};
- $cfg->{errors} = $errors if scalar($errors->@*) > 0;
+
+ $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
next prev parent reply other threads:[~2024-06-04 9:28 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-06-04 9:28 [pve-devel] [PATCH v1 pve-common 0/3] Section Config: Documentation & Code Cleanup Max Carrara
2024-06-04 9:28 ` [pve-devel] [PATCH v1 pve-common 1/3] section config: document package and its methods with POD Max Carrara
2024-06-11 10:46 ` Fabian Grünbichler
2024-06-13 9:14 ` Max Carrara
2024-06-04 9:28 ` [pve-devel] [PATCH v1 pve-common 2/3] section config: update code style Max Carrara
2024-06-04 9:28 ` Max Carrara [this message]
2024-06-11 10:46 ` [pve-devel] [PATCH v1 pve-common 3/3] section config: clean up parser logic Fabian Grünbichler
2024-06-13 9:27 ` Max Carrara
2024-07-24 11:34 ` [pve-devel] applied: [PATCH v1 pve-common 0/3] Section Config: Documentation & Code Cleanup Fiona Ebner
2024-07-24 11:46 ` Fabian Grünbichler
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240604092850.126083-4-m.carrara@proxmox.com \
--to=m.carrara@proxmox.com \
--cc=pve-devel@lists.proxmox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox