Commit 220fb679 authored by Aleksey Midenkov's avatar Aleksey Midenkov

MDEV-28931 Debugger.pm readability fix

setup_boot_args(), setup_client_args(), setup_args() traversing
datastructures on each invocation. Even if performance is not
important to perl script (though it definitely saves some CO2), this
nonetheless provokes some code-reading questions. Reading and
debugging such code is not convenient.

The better way is to prepare all the data in advance in an easily
readable form as well as do the validation step before any further
processing.

Use mtr_report() instead of die() like the other code does.

TODO: do_args() does even more data processing magic. Prepare that
data according the above strategy in advance in pre_setup() if possible.
parent ce7820eb
...@@ -5,6 +5,7 @@ use warnings; ...@@ -5,6 +5,7 @@ use warnings;
use Text::Wrap; use Text::Wrap;
use Cwd; use Cwd;
use My::Platform; use My::Platform;
use mtr_report;
# 1. options to support: # 1. options to support:
# --xxx[=ARGS] # --xxx[=ARGS]
...@@ -105,6 +106,10 @@ EEE ...@@ -105,6 +106,10 @@ EEE
my %opts; my %opts;
my %opt_vals; my %opt_vals;
my $debugger;
my $boot_debugger;
my $client_debugger;
my $help = "\n\nOptions for running debuggers\n\n"; my $help = "\n\nOptions for running debuggers\n\n";
for my $k (sort keys %debuggers) { for my $k (sort keys %debuggers) {
...@@ -161,7 +166,7 @@ sub do_args($$$$$) { ...@@ -161,7 +166,7 @@ sub do_args($$$$$) {
if ($v->{script}) { if ($v->{script}) {
::mtr_tonewfile($vars{script}, subst($v->{script}, %vars)."\n".$script); ::mtr_tonewfile($vars{script}, subst($v->{script}, %vars)."\n".$script);
} elsif ($script) { } elsif ($script) {
die "$k is not using a script file, nowhere to write the script \n---\n$script\n---\n"; mtr_error "$k is not using a script file, nowhere to write the script \n---\n$script\n---";
} }
my $options = subst($v->{options}, %vars); my $options = subst($v->{options}, %vars);
...@@ -186,24 +191,61 @@ sub help() { $help } ...@@ -186,24 +191,61 @@ sub help() { $help }
sub fix_options(@) { sub fix_options(@) {
my $re=join '|', keys %opts; my $re=join '|', keys %opts;
$re =~ s/=s//g; $re =~ s/=s//g;
# FIXME: what is '=;'? What about ':s' to denote optional argument in register_opt()
map { $_ . (/^--($re)$/ and '=;') } @_; map { $_ . (/^--($re)$/ and '=;') } @_;
} }
sub pre_setup() { sub pre_setup() {
my $used; my $used;
my %options;
my %client_options;
my %boot_options;
my $embedded= $::opt_embedded_server ? ' with --embedded' : '';
for my $k (keys %debuggers) { for my $k (keys %debuggers) {
for my $opt ($k, "manual-$k", "boot-$k", "client-$k") { for my $opt ($k, "manual-$k", "boot-$k", "client-$k") {
if ($opt_vals{$opt}) my $val= $opt_vals{$opt};
{ if ($val) {
$used = 1; $used = 1;
if ($debuggers{$k}->{pre}) { if ($debuggers{$k}->{pre}) {
$debuggers{$k}->{pre}->(); $debuggers{$k}->{pre}->();
delete $debuggers{$k}->{pre}; delete $debuggers{$k}->{pre};
} }
if ($opt eq $k) {
$options{$opt}= $val;
$client_options{$opt}= $val
if $embedded;
} elsif ($opt eq "manual-$k") {
$options{$opt}= $val;
} elsif ($opt eq "boot-$k") {
$boot_options{$opt}= $val;
} elsif ($opt eq "client-$k") {
$client_options{$opt}= $val;
}
} }
} }
} }
if ((keys %options) > 1) {
mtr_error "Multiple debuggers specified: ",
join (" ", map { "--$_" } keys %options);
}
if ((keys %boot_options) > 1) {
mtr_error "Multiple boot debuggers specified: ",
join (" ", map { "--$_" } keys %boot_options);
}
if ((keys %client_options) > 1) {
mtr_error "Multiple client debuggers specified: ",
join (" ", map { "--$_" } keys %client_options);
}
$debugger= (keys %options)[0];
$boot_debugger= (keys %boot_options)[0];
$client_debugger= (keys %client_options)[0];
if ($used) { if ($used) {
$ENV{ASAN_OPTIONS}= 'abort_on_error=1:'.($ENV{ASAN_OPTIONS} || ''); $ENV{ASAN_OPTIONS}= 'abort_on_error=1:'.($ENV{ASAN_OPTIONS} || '');
::mtr_error("Can't use --extern when using debugger") if $ENV{USE_RUNNING_SERVER}; ::mtr_error("Can't use --extern when using debugger") if $ENV{USE_RUNNING_SERVER};
...@@ -219,49 +261,20 @@ sub pre_setup() { ...@@ -219,49 +261,20 @@ sub pre_setup() {
sub setup_boot_args($$$) { sub setup_boot_args($$$) {
my ($args, $exe, $input) = @_; my ($args, $exe, $input) = @_;
my $found; do_args($args, $exe, $input, 'bootstrap', $boot_debugger)
if defined $boot_debugger;
for my $k (keys %debuggers) {
if ($opt_vals{"boot-$k"}) {
die "--boot-$k and --$found cannot be used at the same time\n" if $found;
$found="boot-$k";
do_args($args, $exe, $input, 'bootstrap', $found);
}
}
} }
sub setup_client_args($$) { sub setup_client_args($$) {
my ($args, $exe) = @_; my ($args, $exe) = @_;
my $found; do_args($args, $exe, IS_WINDOWS() ? 'NUL' : '/dev/null', 'client', $client_debugger)
my $embedded = $::opt_embedded_server ? ' with --embedded' : ''; if defined $client_debugger;
for my $k (keys %debuggers) {
my @opt_names=("client-$k");
push @opt_names, $k if $embedded;
for my $opt (@opt_names) {
if ($opt_vals{$opt}) {
die "--$opt and --$found cannot be used at the same time$embedded\n" if $found;
$found=$opt;
do_args($args, $exe, IS_WINDOWS() ? 'NUL' : '/dev/null', 'client', $found);
}
}
}
} }
sub setup_args($$$) { sub setup_args($$$) {
my ($args, $exe, $type) = @_; my ($args, $exe, $type) = @_;
my $found; do_args($args, $exe, IS_WINDOWS() ? 'NUL' : '/dev/null', $type, $debugger)
if defined $debugger;
for my $k (keys %debuggers) {
for my $opt ($k, "manual-$k") {
if ($opt_vals{$opt}) {
die "--$opt and --$found cannot be used at the same time\n" if $found;
$found=$opt;
do_args($args, $exe, IS_WINDOWS() ? 'NUL' : '/dev/null', $type, $found);
}
}
}
} }
1; 1;
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment