Re: [pmg-devel] [PATCH pmg-api v2 2/4] api: spamassassin: read local channels

[Thomas Lamprecht <t.lamprecht@proxmox.com>]

On 18.01.21 20:47, Stoiko Ivanov wrote:
>>> +	my $check_channel = sub {
>>> +	    my ($channel) = @_;
>>> +
>>> +	    # see sa-update source:
>>> +	    my $channel_file_base = $channel;
>>> +	    $channel_file_base =~ s/[^A-Za-z0-9-]+/_/g;
>>> +	    my $channelfile = "${sa_update_dir}${channel_file_base}.cf";
>>> +
>>> +	    my $mtime = -1;
>>> +	    my $version = -1;
> $version gets initialized to -1 here...
>>> +	    my $newversion = -1;
>>> +
>>> +	    if (-f $channelfile) {
>>> +		# stat metadata cf file
>>> +		$mtime = (stat($channelfile))[9]; # 9 is mtime
>>> +
>>> +		# parse version from metadata cf file
>>> +		my $metadata = PVE::Tools::file_read_firstline($channelfile);
>>> +		if ($metadata =~ m/\s([0-9]+)$/) {
>>> +		    $version = $1;
>>> +		} else {
>>> +		    warn "invalid metadata in '$channelfile'\n";  
>> I know it was already there, but this  mayresults in a undef comparison
>> when checking `if ($newversion > $version) {` below.
>> I'd do a separate followup with that being fixed, e.g., by either prepending the
>> if check with a `!defined($version) || ...` check or setting it to -1000 or so.
> so it should not be undefined if the metadata is not matching?
> (I quickly tested in on my system - and if I drop the -1 I get a undef
> comparison in the logs - but it works as is)
> or am I missing something?
> 

Ok, my bad then - sorry!