Perl script to set IP settings This is my first go at a Perl script to (semi-) automate setting the correct IP settings. I use it to configure a virtual machine after creation. It's also my first Perl script as a whole, so I'm looking for anyone who can give me pointers towards a better and more efficient coding style.

#!/usr/bin/perl 

use strict;
use warnings;

use Getopt::Long;
use Tie::File;

sub printMissingValues; 
sub gatherInputValues;
sub setValueForField;

my %resulthash = (
    "IPADDR"  =&gt; undef,
    "NETMASK" =&gt; undef,
    "GATEWAY" =&gt; undef,
     "DNS1"    =&gt; undef,
     "DNS2"    =&gt; undef,
);

my $filename = undef;


GetOptions ("filename=s"  =&gt; \$filename,
    "IP=s"        =&gt; \$resulthash{'IPADDR'},
            "NETMASK=s"   =&gt; \$resulthash{'NETMASK'},
            "GATEWAY=s"   =&gt; \$resulthash{'GATEWAY'},
            "DNS1=s"      =&gt; \$resulthash{'DNS1'},
    "DNS2=s"      =&gt; \$resulthash{'DNS2'}) 

or die ("Error in input values");

#gatherInputValues(\%resulthash);

open(my $fd, '&gt;&gt;', "$filename") 
    or die("Cannot open file $filename");

tie my @array, 'Tie::File',  $filename
    or die "Cannot tie file '$filename': $!";

for my $line (@array)
{
   my @fields = split /=/, $line;

    if ($fields[0] eq 'BOOTPROTO') {
         $fields[1] = '"static"';
        $line = join '=', @fields;
        }

    elsif ($fields[0] eq 'IPADDR' ) {
            $line = setValueForField('IPADDR', @fields);
    }

    elsif ($fields[0] eq 'NETMASK') {
            $line = setValueForField('NETMASK', @fields);
    }

    elsif ($fields[0] eq 'GATEWAY') {
            $line = setValueForField('GATEWAY', @fields);
    }

    elsif ($fields[0] eq 'DNS1') {
            $line = setValueForField('DNS1', @fields);
    }

    elsif ($fields[0] eq 'DNS2') {
            $line = setValueForField('DNS2', @fields);
    }
}

untie @array;

addMissingValues(\%resulthash);

close($fd);

##functions##

sub addMissingValues {
    my $hash = shift @_;

    while(my($key, $value) = each %{$hash}) {
        if (defined ($value)) {
            createNewField($key, $value);
        }
    }
}

 sub createNewField {
    my $key = shift @_;
    my $value = shift @_;
    print $fd (join '=', $key, $value);
    print $fd ("\n");
}

sub setValueForField {
    my $fieldname = shift;
    my @fields = shift;

    $fields[1] = $resulthash{$fieldname};

    return  (join '=', @fields);
}

sub gatherInputValues {

    my $result_hash = shift @_;

    print "IP4-address: ";
    $result_hash-&gt;{'IPADDR'} = &lt;STDIN&gt;;
    print "subnet-mask: ";
    $result_hash-&gt;{'NETMASK'} = &lt;STDIN&gt;;
    print "gateway: ";
    $result_hash-&gt;{'GATEWAY'} = &lt;STDIN&gt;;
    print "dns1: ";
    $result_hash-&gt;{'DNS1'} = &lt;STDIN&gt;;
    print "dns2: ";
    $result_hash-&gt;{'DNS2'} = &lt;STDIN&gt;;
}


This is the file the script operates on (standard configuration script for CentOS systems - /etc/sysconfig/network-scripts/ifcfg-etho)

TYPE="Ethernet"
PROXY_METHOD="none"
BROWSER_ONLY="no"
BOOTPROTO="static"
DEFROUTE="yes"
IPV4_FAILURE_FATAL="no"
IPV6INIT="yes"
IPV6_AUTOCONF="yes"
IPV6_DEFROUTE="yes"
IPV6_FAILURE_FATAL="no"
IPV6_ADDR_GEN_MODE="stable-privacy"
NAME="eth0"
UUID="7cabbc98-e67d-4a64-a132-7e8bcbeb579b"
DEVICE="eth0"
ONBOOT="yes
There's no need to predeclare the subs (sub whatever;).
There's no need to assign the initial values to %resulthash.
my $filename; does the same as my $filename = undef;. There's no need to specify the undef.
Hash keys are autoquoted if they are simple (i.e. follow the same rules as variable name). Instead of $resulthash{'IPADDR'} you can type just $resulthash{IPADDR}.
shift in a sub operates on @_. So you can just write my $hash = shift;.
Instead of shifting @_ several times, assign all the variables at the same time: my ($key, $value) = @_;.


Plus, you're comparing $fields[0] to various strings, maybe a "dispatch table" would be a bit clearer. See my version of the script:

#!/usr/bin/perl
use strict;
use warnings;

use Getopt::Long;
use Tie::File;

my %resulthash;
my $filename;

GetOptions ('filename=s' =&gt; \$filename,
            'IP=s'       =&gt; \$resulthash{IPADDR},
            'NETMASK=s'  =&gt; \$resulthash{NETMASK},
            'GATEWAY=s'  =&gt; \$resulthash{GATEWAY},
            'DNS1=s'     =&gt; \$resulthash{DNS1},
            'DNS2=s'     =&gt; \$resulthash{DNS2})
or die 'Error in input values';

open my $fd, '&gt;&gt;', $filename
    or die "Cannot open file $filename";

tie my @array, 'Tie::File',  $filename
    or die "Cannot tie file '$filename': $!";

for my $line (@array) {
    my @fields = split /=/, $line;

    { BOOTPROTO =&gt; sub { $fields[1] = '"static"';
                         $line = join '=', @fields; },
      IPADDR    =&gt; sub { $line = setValueForField('IPADDR', @fields); },
      NETMASK   =&gt; sub { $line = setValueForField('NETMASK', @fields); },
      GATEWAY   =&gt; sub { $line = setValueForField('GATEWAY', @fields); },
      DNS1      =&gt; sub { $line = setValueForField('DNS1', @fields); },
      DNS2      =&gt; sub { $line = setValueForField('DNS2', @fields); },
    }-&gt;{ $fields[0] }-&gt;();
}

untie @array;

addMissingValues(\%resulthash);

close $fd;

sub addMissingValues {
    my $hash = shift;

    while (my ($key, $value) = each %$hash) {
        createNewField($key, $value) if defined $value;
    }
}

sub createNewField {
    my ($key, $value) = @_;
    print {$fd} join '=', $key, $value;
    print {$fd} "\n";
}

sub setValueForField {
    my ($fieldname, @fields) = @_;

    $fields[1] = $resulthash{$fieldname};

    return join '=', @fields;
}


You can also keep the dispatch table outside of the loop, but you need to pass parameters to the callbacks.

my %DISPATCH = (
    BOOTPROTO =&gt; sub { $_[1] = '"static"';
                       join '=', @_ },
    IPADDR    =&gt; sub { setValueForField('IPADDR', @_) },
    NETMASK   =&gt; sub { setValueForField('NETMASK', @_) },
    GATEWAY   =&gt; sub { setValueForField('GATEWAY', @_) },
    DNS1      =&gt; sub { setValueForField('DNS1', @_) },
    DNS2      =&gt; sub { setValueForField('DNS2', @_) },
);

for my $line (@array) {
    my @fields = split /=/, $line;
    my $sub = $DISPATCH{ $fields[0] };
    $line = $sub-&gt;( @fields) if $sub;
}