comment on

Let me start by cautioning you to not run your code directly on your system hosts file. It's much safer to take a local copy, run your script on the local copy, then do a "diff" with the original as a check, then copy it over manually. Do it this way at least until you are certain your script is sound and bullet-proof ... which it is not currently, as described in detail below.

It is a bit hard to critique your code because you have not clearly stated the intent. From a cursory scan of your code I will assume that the intent is:

Update the hosts file in place. If the hosts file does not contain the ip address 10.10.1.2 do nothing. If it contains a line starting with host 10.10.1.2, comment it out. If it contains a commented out line starting with host 10.10.1.2, uncomment it. It is unclear to me whether your intent is a specific ip address, namely 10.10.1.2, or all ip addresses that start with 10.10.1.2 (which is what your code is actually doing). Please let us know.

In any case, most of my comments are general and independent of your specific intent.

First, you should always start your Perl code with:

use strict;
use warnings;
[download]

You forgot the "use warnings". Adding that reveals that your code throws a warning:

Use of uninitialized value in string eq
[download]

in this line:

map {if($_ =~ /^(#)?(10\.10\.1\.2.*)/){$_ = $1 eq '#' ? $2 : "#$2";}} 
+@Output;
[download]

Looking at this line of code gives me a headache. You should aim for simplicity, clarity and correctness in your code. I further noticed from running your program that the above line contains a bug in that it loses the terminating newline.

Staring at the offending line of code, we see that you are using map in void context -- which is generally frowned upon. You see, map returns a list, yet you are throwing it away! According to Perl luminary tchrist (Tom Christiansen), this "makes for muddled code".

Next, we see you are modifying a list with map, which is also frowned upon. Effective Perl Programming item 20, "Use foreach, map and grep as appropriate", provides a nice summary of when to use foreach, map and grep:

Use foreach to iterate read-only over each element of a list
Use map to create a list based on the contents of another list
Use foreach to modify elements of a list
Use grep to select elements in a list

Here you are modifying the elements of a list and so should use foreach in preference to map.

Finally, be aware that the way you've written the code is unsound in that it offers the potential for permanent (and possibly silent) data loss. Consider what happens if your line:

print Out @Output;
[download]

fails. You're not checking print's return value, so you may not even be aware that it's failed. If it fails, your hosts file will be corrupted. This is an important file. If you don't have a backup copy, you will have suffered permanent data loss. This topic is discussed at length in:

In summary then, I would write your program something like this:

use strict;
use warnings;

# Update $fname re-runnably.
# $fcontents is either a string or a reference to an array of lines.
sub atomic_update
{
   my ( $fname, $fcontents ) = @_;
   print "updating '$fname'...";
   my $bak = $fname . $$ . '.tmp';
   -e $bak and ( unlink($bak) or die "error: unlink '$bak': $!" );
   open( my $fh, '>', $bak ) or die "error: open '$bak': $!";
   print {$fh} ref($fcontents) ? @{$fcontents} : $fcontents
      or die "error: print '$bak': $!";
   close($fh) or die "error: close '$bak': $!";
   rename( $bak, $fname ) or die "error: rename '$bak' '$fname': $!";
   print "done.\n";
}

my $hostfile = shift || "C:/Windows/System32/drivers/etc/hosts";
print "hostfile : $hostfile\n";

open(my $In, '<', $hostfile) or die "error: open '$hostfile': $!";
my @Output = <$In>;
close $In;

# If 10.10.1.2 is commented out, uncomment it.
# If it is not commented out, comment it out.
my $nchanges = 0;
for (@Output)
{
   if ( s/^#(?=10\.10\.1\.2\b)// )
   {
      ++$nchanges;
      next;
   }
   if ( s/^(?=10\.10\.1\.2\b)/#/ )
   {
      ++$nchanges;
      next;
   }
}
if ($nchanges > 0)
{
   atomic_update( $hostfile, \@Output );
}
print "There were $nchanges changes made to file '$hostfile'\n";
[download]

Please note that my code is checking specifically for ip address 10.10.1.2, not anything starting with 10.10.1.2. To instead change ip addresses starting with 10.10.1.2, simply lose the "\b" in the regexes, like this:

   s/^#(?=10\.10\.1\.2)//
   s/^(?=10\.10\.1\.2)/#/
[download]

In reply to Re: Read in hostfile, modify, output by eyepopslikeamosquito
in thread Read in hostfile, modify, output by razmeth

Posts are HTML formatted. Put <p> </p> tags around your paragraphs. Put <code> </code> tags around your code and data!

Titles consisting of a single word are discouraged, and in most cases are disallowed outright.

Read Where should I post X? if you're not absolutely sure you're posting in the right place.

Please read these before you post! —

Posts may use any of the Perl Monks Approved HTML tags:

a, abbr, b, big, blockquote, br, caption, center, col, colgroup, dd, del, details, div, dl, dt, em, font, h1, h2, h3, h4, h5, h6, hr, i, ins, li, ol, p, pre, readmore, small, span, spoiler, strike, strong, sub, summary, sup, table, tbody, td, tfoot, th, thead, tr, tt, u, ul, wbr

You may need to use entities for some characters, as follows. (Exception: Within code tags, you can put the characters literally.)

	For:		Use:
	&		`&`
	<		`<`
	>		`>`
	[		`[`
	]		`]`

Link using PerlMonks shortcuts! What shortcuts can I use for linking?

See Writeup Formatting Tips and other pages linked from there for more info.