Gabor Szabo wrote:
It has biten me so let's make sure invalid --gc options are not allowed.
Index: lib/Parrot/Configure/Options.pm
===================================================================
--- lib/Parrot/Configure/Options.pm (revision 20963)
+++ lib/Parrot/Configure/Options.pm (working copy)
@@ -48,6 +48,9 @@
}
$data->{$key} = $value;
}
+ if ($data->{gc} and $data->{gc} !~ /^(gc|libc|malloc|malloc-trace)$/) {
+ croak "Invalid value for 'gc' argument.\nType perl
Configure.pl --help to see available values";
+ }
if (@short_circuits_seen) {
# run all the short circuits
foreach my $sc (@short_circuits_seen) {
Gabor,
While I appreciate the intent of your patch, it's not focused where it
needs to be.
lib/Parrot/Configure/Options.pm's sole purpose is to provide
functionality needed for processing of command-line options by both
Configure.pl and tools/dev/reconfigure.pl. It has nothing to do with
deciding whether a user-supplied *value* for a particular option is
valid or not. The validity of a user-supplied value for an option must
be determined by the particular configuration step in which that
option's value is consulted.
I would recommend that you reformulate your patch as follows:
1. Change the description of the --gc option provided in
lib/Parrot/Configure/Options/Conf.pm's print_help() subroutine. This is
the usage message for Configure.pl. That description currently occurs
on lines 175-176 of that package.
2. Modify config/auto/gc.pm to have configuration step auto::gc fail if
any invalid option is provided. (I did a quick grep on config/ and this
is the only place I saw the 'gc' option handled -- but I might have
gotten confused by the many instances of 'gcc'.) A statement along the
lines of 'return unless $gc =~ /list|of|valid|options/;' around line 62
of that package would probably be the way to go.
Thank you very much.
Jim Keenan
