[pve-devel] [PATCH common] Fix 2339: Handle multiple blank lines correctly

[Thomas Lamprecht]

On 27.08.19 09:01, Fabian Ebner wrote:
> Otherwise two blank lines between sections cause the loop to end prematurely.
> 
> Signed-off-by: Fabian Ebner <f.ebner at proxmox.com>
> ---
>  src/PVE/SectionConfig.pm | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/src/PVE/SectionConfig.pm b/src/PVE/SectionConfig.pm
> index dcecce6..1bb285f 100644
> --- a/src/PVE/SectionConfig.pm
> +++ b/src/PVE/SectionConfig.pm
> @@ -308,7 +308,10 @@ sub parse_config {
>  	}
>      };
>  
> -    while (my $line = &$nextline()) {
> +    while (@lines) {
> +	my $line = &$nextline();
> +	next unless $line;

In general it looks like it would do the job, so good work.

Some style nits and a possible alternative:

We do not use "unless" in our projects, it's a perl-ism and can be a bit
confusing. While the use here could be OK, I'd like to keep this conform
to our Perl Style[0]

[0]: https://pve.proxmox.com/wiki/Perl_Style_Guide

AFAICT, the main issue here is that an empty line '' evaluates as false-y,
so an alternative could be (I did not test it):

while ((my $line = &$nextline()) != undef) {

or, to convert it to our newer code-refernce call syntax:

while ((my $line = $nextline->()) != undef) {
( $code->() vs. &$code() )

But just as an idea, if you prefer a:

next if !$line;

that would be totally OK too.

> +
>  	my $errprefix = "file $filename line $lineno";
>  
>  	my ($type, $sectionId, $errmsg, $config) = $class->parse_section_header($line);
>