Fabio/ddclient
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

read_recap: Reference %recap directly (for readability)

Browse source 
There's no need to pass a reference to `%recap` as an argument when
that is the only way `read_recap` is ever used.
This commit is contained in:
Richard Hansen 2024-08-22 02:30:21 -04:00
parent 31740006d0
commit 35cbc8d200
2 changed files with 15 additions and 17 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

30
ddclient.in
View file

@ -1286,7 +1286,7 @@ sub main {
%opt = %saved_opt; %opt = %saved_opt;
read_config(opt('file'), \%config, \%globals); read_config(opt('file'), \%config, \%globals);
init_config(); init_config();
read_recap(opt('cache'), \%recap); read_recap(opt('cache'));
print_info() if opt('debug') && opt('verbose'); print_info() if opt('debug') && opt('verbose');
$daemon = opt('daemon'); $daemon = opt('daemon');
@ -1626,20 +1626,18 @@ sub write_recap {
###################################################################### ######################################################################
sub read_recap { sub read_recap {
my $file = shift; my $file = shift;
my $config = shift;
my $globals = {}; my $globals = {};
%{$config} = (); %recap = ();
return if !(-e $file); return if !(-e $file);
my %saved = %opt; my %saved = %opt;
%opt = (); %opt = ();
$saved_recap = _read_config($config, $globals, "##\\s*$program-$version\\s*", $file); $saved_recap = _read_config(\%recap, $globals, "##\\s*$program-$version\\s*", $file);
%opt = %saved; %opt = %saved;
# TODO: Why is this iterating over `keys(%recap)`? Shouldn't it iterate over `keys(%config)` # TODO: Why is this iterating over `keys(%recap)`? Shouldn't it iterate over `keys(%config)`
# instead? # instead?
for my $h (keys(%recap)) { for my $h (keys(%recap)) {
# TODO: `$config->{$h}` is guaranteed to exist because `$config` is `\%recap` and we're # TODO: `$recap{$h}` is guaranteed to exist because we're iterating over `keys(%recap)`.
# iterating over `keys(%recap)`. next if !exists($recap{$h});
next if !exists($config->{$h});
# Only status variables are copied from `%recap` into `%config` because the recap should # Only status variables are copied from `%recap` into `%config` because the recap should
# not change the configuration. See the documenting comments for `%recap`. # not change the configuration. See the documenting comments for `%recap`.
# TODO: Hard-coding this list impairs readability and maintainability. In particular, # TODO: Hard-coding this list impairs readability and maintainability. In particular,
@ -1648,15 +1646,15 @@ sub read_recap {
# variables. # variables.
for my $v (qw(atime mtime wtime ipv4 ipv6 status-ipv4 status-ipv6 for my $v (qw(atime mtime wtime ipv4 ipv6 status-ipv4 status-ipv6
warned-min-interval warned-min-error-interval)) { warned-min-interval warned-min-error-interval)) {
# TODO: This is a no-op because `$config` is `\%recap`. I believe the original intent # TODO: This is a no-op. I believe the original intent was to copy values that convey
# was to copy values that convey update status from `%recap` into `%config` so that # update status from `%recap` into `%config` so that ddclient can be restarted and
# ddclient can be restarted and resume where it left off and not violate # resume where it left off and not violate `min-interval`, `min-error-interval`, etc.
# `min-interval`, `min-error-interval`, etc. For the short term, this should copy the # For the short term, this should copy the values into `%config`, not `%recap`. In the
# values into `%config`, not `%recap`. In the long term, nothing should be copied from # long term, nothing should be copied from `%recap` into `%config` because `%config` is
# `%recap` into `%config` because `%config` is not the semantically appropriate place # not the semantically appropriate place to record update status. Code that currently
# to record update status. Code that currently reads/sets `$config{$h}{'status-ipv4'}` # reads/sets `$config{$h}{'status-ipv4'}` (and friends) should instead access
# (and friends) should instead access `$recap{$h}{'status-ipv4'}` directly. # `$recap{$h}{'status-ipv4'}` directly.
$config->{$h}{$v} = $recap{$h}{$v} if exists($recap{$h}{$v}); $recap{$h}{$v} = $recap{$h}{$v} if exists($recap{$h}{$v});
} }
} }
} }

2
t/read_recap.pl
View file

@ -179,7 +179,7 @@ for my $tc (@test_cases) {
local %ddclient::config; local %ddclient::config;
$ddclient::config{$_} = {%{$want_config{$_}}} for keys(%want_config); $ddclient::config{$_} = {%{$want_config{$_}}} for keys(%want_config);
ddclient::read_recap($cachef->filename(), \%ddclient::recap); ddclient::read_recap($cachef->filename());
TODO: { TODO: {
local $TODO = $tc->{want_TODO}; local $TODO = $tc->{want_TODO};