hacker has asked for the wisdom of the Perl Monks concerning the following question:

I have an interesting issue with calling system() and passing it about 2-dozen arguments and parameters in a script. This script is actually an email reflector, and accepts an email template, parses out key=value from the template (via Config::IniFiles), sets some scalars to the values found, and then "stacks" those arguments into an array, and passes it off to the system() command in list-mode. So far, so good.. until bad input is given. Here's an example of the template:
[template] url = http://www.wired.com/news_drop/palmpilot/ AvantGo = No maxdepth = 2 bpp = 4 compression = zlib title = Wired News no_url_info = True beamable = False ... # about 15 other arguments possible here [end_template]

There are several ways the arguments can be passed, depending on the nature of the arguments. Some arguments to the system command being called are boolean, on and off. An example is beamable = 1, which is associated with the --beamable argument that the system binary expects. If beamable = 0, then --not-beamable is passed to the system binary.

Another one is no_url_info = 1, which translates to a --no-urlinfo argument being passed to the system binary. If no_url_info = 0 in the template, then --no-urlinfo is not sent as an argument (i.e. undef), because there is no complementary "false" argument string for this binary. In code, the tests for this look like the following:

# Thanks for the help here 'demerphq' and 'hiseldl' if (defined $beamable && ($beamable =~ /1|true|yes/i)) { $beamable = '--beamable'; $beamable_msg = "Yes, document is beamable"; } elsif (defined $beamable && ($beamable =~ /0|false|no/i)) { $beamable = '--not-beamable'; $beamable_msg = "No, document is copy-protected"; } else { $beamable_msg = "Unspecified, defaults to beamable"; } print LOG " Beamable: $beamable\n"; # Test for no_url_info = value in the template if (defined $no_url_info && ($no_url_info =~ /1|true|yes/i)) { $no_url_info = '--no-urlinfo'; $no_url_info_msg = "No, url information disabled"; } elsif (defined $no_url_info && ($no_url_info =~ /0|false|no/i)) { $no_url_info = (); $no_url_info_msg = "Yes, url information shown"; } else { $no_url_info_msg = "Unspecified, url information shown"; } print LOG " url Info: $no_url_info\n";

Then I stack these arguments, and pass them to the system via system() in list-mode as follows:

my $systemcmd = "/usr/bin/appname"; my @systemargs = ('-p', $workpath, '-P', $workpath, '-H', $url, ($maxdepth < 3 ? "--maxdepth=$maxdepth" : "--maxdepth=2"), $bpp ? "--bpp=$bpp" : (), # thanks to 'tye' ($compression ? "--${compression}-compression" : ''), '-N', $title, $beamable ? $beamable : (), $no_url_info ? $no_url_info : (), '-V1', '-f', "$workpath/$md5file"); print LOG "Command: $systemcmd @systemargs\n"; system($systemcmd, @systemargs);

So far, so good, but here's where I need some extra eyes: As I add more arguments to compensate for everything this binary can accept, bools, values, and so on, I am going to need to test them for the same true, false, undef, and output the right value to the system call accordingly.

Each of the two tests above were 10 lines, not including the line for the ternary in the system call itself. Across about 2-dozen possible arguments to the system call, this could baloon very fast, and be quite a lot of duplicated code, except for the actual key name itself. I'd like to eliminate that duplication as much as possible.

I'm thinking that this could be done with a hash of some sort, which contains the argument name and the values it can take, such as:

my @args = ({s_arg =>'beamable', # argument name t_arg =>'--beamable', # true argument t_msg =>'Document is beamable', # true message f_arg =>'--not-beamable', # false argument f_msg =>'Document is protected},# false message s_arg =>'urlinfo', t_arg =>'', t_msg =>'Yes, url information provided', f_arg =>'--no-urlinfo', f_msg =>'No, no url information provided'})

At some point, I also need to test that the value passed in the template was either true, false, or undef, and then pull the right member out of the hash, and send it to the system call.

Is there an easy way to "walk the hash" and set options accordingly, based on what values are found in the template?

Replies are listed 'Best First'.
Re: Cleansing and stacking arguments before calling system()
by sauoq (Abbot) on Sep 13, 2002 at 21:37 UTC

    It would probably help to generalize the checking with a subroutine. This might help get you started. It would take the configuration setting and a hashref with all the relevant info. It would return the option and the message.

    sub optionize { my ($config_setting, $opthash) = @_; my $option_name = $opthash->{option}; if (defined $config_setting && ($config_setting =~ /1|true|yes/i)) +{ return( "--$option_name", $opthash->{msg_yes} ); } elsif (defined $config_setting && ($config_setting =~ /0|false|no +/i)) { return( "--not-$option_name", $opthash->{msg_no} ); } else { return( undef, $opthash->{msg_default} ); } }

    If you are going to keep a hash of all of the argument stuff, I suggest you make it an HoH where the subhashes are keyed by some canonical name for the argument. Lots of "F-arg" and "t_arg" and "t_msg" and "f_msg" keys are going to get nasty.

    I also suggest that you think about using READMORE tags in long questions like that.

    -sauoq
    "My two cents aren't worth a dime.";
    
Re: Cleansing and stacking arguments before calling system()
by fglock (Vicar) on Sep 13, 2002 at 20:55 UTC

    I guess you can do this job by playing some tricks with Getopt::Long. It will even give you some error checking. But I'm not sure if it is the right tool.

Re: Cleansing and stacking arguments before calling system()
by Aristotle (Chancellor) on Sep 14, 2002 at 04:02 UTC
Re: Cleansing and stacking arguments before calling system()
by BrowserUk (Patriarch) on Sep 14, 2002 at 05:42 UTC

    This is far from complete, but as the output at the bottom shows, the basic logic is inplace and functional. The regexen will require scrutiny as will the treatment of missing required values and other error handling.

    I've written a simplistic parser for the purposes of testing. You my want to reject this in favour of something better.

    Hope it at least demonstrates an approach to the problem.

    #! perl -sw use strict; my %options = ( url => { required => 1, type => 'string', mask => qr/^(\S{1}.*?\S{1})$/io, map => sub { (my $arg = $_[0]) =~ s/(.*)/--url="$1" +/io; return $arg; }, default => undef, }, title => { required => 0, type => 'string', mask => qr/^(\S{1}.*?\S{1})$/io, map => sub { (my $arg = $_[0]) =~ s/(.*)/--title="$ +1"/io; return $arg; }, default => undef, }, AvantGo => { required => 0, type => 'boolean', mask => qr/^(0|1|y|n|yes|no|t|f|true|false)$/io, # NOTE: Longer option MUST come before shorter equ +ivalents in substitutions. maptrue => sub { (my $arg = $_[0]) =~ s/(1|y|yes|true|t +)/--avantgo/io; return $arg; }, mapfalse => sub { (my $arg = $_[0]) =~ s/(0|no|n|false|f)/- +-no_avantgo/io; return $arg; }, default => '--no_avantgo', }, maxdepth => { required => 1, type => 'numeric', mask => qr/^(\d+)$/, map => sub { (my $arg = $_[0]) =~ s/(\d+)/--maxdept +h=$1/o; return $arg; }, default => '--maxdepth=2', }, bbp => { required => 0, type => 'numeric', mask => qr/^(1|2|4|8|32)$/, map => sub { (my $arg = $_[0]) =~ s/(\d+)/--bbp=$1/ +o; return $arg; }, default => '--bbp=4', }, compression => { required => 1, type => 'numeric', mask => qr/^(zlib|zip|gzip|tar)$/i, map => sub { (my $arg = $_[0]) =~ s/(\w+)/--compres +sion=$1/io; return $arg; }, default => '--compression=zlib', }, no_url_info => { required => 0, type => 'boolean', mask => qr/^(0|1|y|n|yes|no|t|f|true|false)$/i, maptrue => sub { (my $arg = $_[0]) =~ s/(1|yes|y|true|t +)/--no_url_info/io; return $arg; }, mapfalse => sub { (my $arg = $_[0]) =~ s/(0|no|n|false|f)// +io; return $arg; }, default => '', }, beamable => { required => 0, type => 'boolean', mask => qr/^(0|1|y|n|yes|no|t|f|true|false)$/i, maptrue => sub { (my $arg = $_[0]) =~ s/(1|yes|y|true|t +)/--beamable/io; return $arg }, mapfalse => sub { (my $arg = $_[0]) =~ s/(0|no|n|false|f)/- +-not-beamable/io; return $arg; }, default => '--beamable', }, ); my @rawInput = <DATA>; my $pInput = parseInput( \@rawInput ); # builds and returns ref to has +h of input parameters my @exeArgs = (); my $fatalError = 0; *LOG = *STDERR; foreach my $option (keys %options) { print LOG 'Option ', $option; if( exists $pInput->{$option} ) { # the option was +present print LOG ' found: "'; if (defined $pInput->{$option}) { # and they suppli +ed a value print LOG $pInput->{$option}, '" '; if ( $pInput->{$option} =~ $options{$option}{mask} ) { #pa +sses the mask print LOG ' passed '; if ( $options{$option}{type} eq 'boolean' ) { # try and extract a true switch my $arg = $options{$option}{maptrue}( $pInput->{$o +ption} ); # extract a false switch if that failed $arg = $options{$option}{mapfalse}( $pInput->{$opt +ion} ) if ( $arg eq $pInput->{$option} ); print LOG $/, 'using switch "', $arg, '"', $/; push @exeArgs, $arg; } else { # type = string or numeric my $arg = $options{$option}{map}( $pInput->{$optio +n} ); print LOG $/, 'using switch "', $arg, '"', $/; push @exeArgs, $arg; } } else { # Handle supplied value invalid print LOG 'failed '; if ( $options{$option}{required} ) { # they must suppl +y a valid value print LOG 'request rejected', $/; warn "bad args\n"; $fatalError = 1; next; } else { my $arg = $options{$option}{default}; print LOG 'using default "', $arg, '"', $/; push @exeArgs, $arg; } } } else { # Handle defined key without value print LOG 'no value supplied '; if ( $options{$option}{required} ) { print 'required value not supplied, request rejected', + $/; warn "bad args\n"; $fatalError = 1; next; } else { my $arg = $options{$option}{default}; print 'using default "', $arg, '"', $/; push @exeArgs, $arg; } } } else { print LOG 'Required option ', $option, ' missing '; if ( $options{$option}{required} ) { # Handle missing required option print LOG '- rejecting request', $/; } else { my $arg = $options{$option}{default}; print LOG '- using default "', $arg, '"', $/; push @exeArgs, $arg; } } } if ( $fatalError ) { print LOG 'command not issued', $/; # do something about it. } else { # better handling here. print 'Issuing command with arguements:'.$/; print $_.$/ for @exeArgs; print LOG 'command returned: ', system( 'command', @exeArgs ); } exit; sub parseInput{ # Expecting a single parm die 'parseInput: Invalid parm(s) supplied'.$/ if @_ != 1 or ref($_ +[0]) ne 'ARRAY'; my ( $pRawInput ) = shift; my %input; $input{extraneous} = (); # set up an empty array for lines we cann +ot parse. for ( @{$pRawInput} ) { chomp; # print "'$_'\n"; my $keyseen = ''; if ( m/^\s*?(\S+)\s*?=\s*?(\S{1}.*?)$/ ) { # k +ey = somestuff \n $keyseen = $1; $input{$1} = $2 if $2; # a +(possibly partial) value found print "found key: '$1' value: '$2'\n"; } elsif ( m/^\s*?(\S+)\s*?=$/ ) { # key = \n + WARNING This regex not exercised!!!! $keyseen = $1; $input{$1} = ''; # no value + yet print "no value for key: '$keyseen' yet\n"; } elsif ($keyseen) { # $input{$keyseen} .= $_; # assume i +t part of the previous keys value print "Adding '$_' to the value of key: '$keyseen'\n"; } else { # line contains no key, and no key yet seen (or blank l +ine). push @{$input{extraneous}}, $_ if ! m/^$/; # discard blank +s print "Added '$_' to extraneuos array\n"; } } print "\nparsing completed \n\n"; return \%input; } __DATA__ url = http://www.wired.com/news_drop/palmpilot/ AvantGo = No maxdepth = 2 bpp = 4 compression = zlib title = Wired News no_url_info = True beamable = False

    The following is a run on the limited test data supplied.

    C:\test>perl 197724.pl found key: 'url' value: 'http://www.wired.com/news_drop/palmpilot/' found key: 'AvantGo' value: 'No' found key: 'maxdepth' value: '2' found key: 'bpp' value: '4' found key: 'compression' value: 'zlib' found key: 'title' value: 'Wired News' found key: 'no_url_info' value: 'True' found key: 'beamable' value: 'False' parsing completed Option maxdepth found: "2" passed using switch "--maxdepth=2" Option url found: "http://www.wired.com/news_drop/palmpilot/" passed using switch "--url="http://www.wired.com/news_drop/palmpilot/"" Option beamable found: "False" passed using switch "--not-beamable" Option title found: "Wired News" passed using switch "--title="Wired News"" Option bbpRequired option bbp missing - using default "--bbp=4" Option no_url_info found: "True" passed using switch "--no_url_info" Option AvantGo found: "No" passed using switch "--no_avantgo" Option compression found: "zlib" passed using switch "--compression=zlib" Issuing command with arguements: --maxdepth=2 --url="http://www.wired.com/news_drop/palmpilot/" --not-beamable --title="Wired News" --bbp=4 --no_url_info --no_avantgo --compression=zlib The system cannot execute the specified program. command returned: 256 C:\test>

    Well It's better than the Abottoire, but Yorkshire!

        Really? And there was me thinking I was knocking up a quick proof-of-concept script. Still, I learnt a few things along the way, which is why I try to answer almost all the questions posted here in the first place. I only post the results if I *think* I have something that might be useful.

        Re: Config::Inifiles. As hacker stated, he has been using that module, but going by the code he posted, it seems as though the keys parsed get supplied back to the caller as individual variables (eg. $bpp = 4 ) rather than as a collection? This would appear to be the main cause of hacker's coding troubles. It's much easier to avoid the

        if(exists key) { if defined key) { if value supplied { if(value is valid) { if(value eq this) { use this } elsif (value eq that) { do that } else { yell } } else { use default } } else if( value is required ) { do something } else do something else } } else { key not supplied... }

        for every key, if they are in a collection.

        Note:I haven't looked at the modules you mention, I'm just going by what I see.


        Well It's better than the Abottoire, but Yorkshire!