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.

First, you should always start your Perl code with:

use strict;
use warnings;
[download]

Use of uninitialized value in string eq
[download]

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

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

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]

• Perl Best Practices book: is this one a best practice or a dodgy practice?
• Re-runnably editing a file in place

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]

Also, (#)? would be more readable as (#?) , as you want to match the octothorpe if present, not to match it always, but maybe zero times.

You correctly used a lexical filehandle for the input. Use a lexical filehandle for the output, too.

The preferred way of processing files, though, is to read them line by line if possible. You need to print to a temporary file, as you can't read a file and print to it at the same time.

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

my $file = 'C:/Windows/System32/drivers/etc/hosts';
my %replace = ( '#' => q(), '' => '#' );

open my $In,  '<', $file or die $!;
open my $Out, '>', "$file.new" or die $!;
while (<$In>) {
    s/(#?)(10\.10\.1\.2)/$replace{$1}$2/;
    print {$Out} $_;
}
close $Out or die $!;
rename "$file.new", $file or die $!;
[download]

($q=q:Sq=~/;[c](.)(.)/;chr(-||-|5+lengthSq)`"S|oS2"`map{chr |+ord
}map{substrSq`S_+|`|}3E|-|`7**2-3:)=~y+S|`+$1,++print+eval$q,q,a,
[download]

As others have said, this line,

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

Do not make the mistake of assuming that fewer lines of Perl code means more efficient or "better" code. This map is an implied foreach loop whether you want to write the source code that way or not.

I prefer to translate $1, $2, etc into named variables when possible. Compared with firing up the regex engine, this is very "cheap" CPU wise. Also, white space consumes no MIPs! Space things out and write something that you will understand 2 years from now.

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

my @hosts = ("10.10.1.2   # some comment ...\n", 
             "#10.10.1.56\n", 
             "14.1.2.89\n");
             
my $pattern = '10.10.1';            

foreach my $line (@hosts)
{
  if ( my ($comment, $rest) = $line =~ /^(#)?($pattern.*\n)/)
   {
      if (defined $comment)
      {
         $line = $rest;   # delete the beginning # comment char
      }
      else
      {
        $line = "#$rest"; # add a beginning # comment char  
      }
   }
}
print @hosts;
__END__

#10.10.1.2   # some comment ...
10.10.1.56
14.1.2.89
[download]

The idea of over-writing the original file with the modified contents is not so good. That will work most of the time, but I would make a temp file of some sort, write to that, then rename the temp file to the host file name. If for nothing else, this will help with the debugging!

There is no truly "atomic" operation on the file system - something can always go wrong within a fileop. Good system code plans for that and also makes the "window of vulnerability" as short as possible.

There is no truly "atomic" operation on the file system

From the Open Group Specification of rename:

That specification requires that the action of the function be atomic

That is, the rename(1) system call is required to be atomic on all Unix systems. As tye points out, rename is also atomic on Windows.

In practice, on Unix and Windows, so long as both files reside on the same OS file system, you should be able to rely on the Perl rename function being atomic.

Geez, I see that my poorly explained comment about atomic operations generated some discussion.

To digress a bit, Wiki Linearizability contains a good discussion about what "atomic" means. I'll just quote part: Atomicity is a guarantee of isolation from concurrent processes. Additionally, atomic operations commonly have a succeed-or-fail definition, they either successfully change the state of the system, or have no apparent effect.

In the case of a single rename(), as long as the system software and hardware work as expected, a single rename can be considered atomic. There are ways (like system reset (hardware,software or human induced) or power supply transient) that can mess things up.

To replace an important file with a new version, first get the new file onto the system in its entirety. Now a quick two step dance happens. At a minimum, rename the existing file to something else, probably some sort of .bak. Then as soon as possible, rename the new file to what the existing file's name was. With everything working perfectly, there is a brief interval where "File" does not exist - that happens between the 2 renames. Any open() attempted during that very short interval will fail.

At the lowest level (disk writing hardware), a disk write operation is not "atomic" and can fail this test: either successfully change the state of the system, or have no apparent effect. When writing a sector on the HD, the sequence: a) Old data, b)No data(the write is occuring), c)New data occurs. If something goes wrong during step (b), then we are left with unknown garbage. Even the very fast rename function goes through these 3 steps.

Summary: Rename is the fastest and best function to use for replacing files. Get the new file completely ready in advance. Do not mess around with writing the new file line by line - that is too slow and opens up corruption issues.

If you want to keep it short i'd go with a oneliner: -i as described in perlrun let you to specify an additional extension for the backup file.

Also note that, as you are running into an unfriendly OS, you can lost you if use Perl 32bit instead of Perl 64bit: read Re: Cannot execute external process: Win32 Win64 SysWOW64 and System32 redirection and you'll know why i use %WINDIR%\Sysnative\drivers\etc\hosts weird syntax. Other infos at my homenode

The following oneliner comment/uncomment any 10.10.1.2* association in your file

perl -i.OLD -nle "$re=qr(^(#?)\s*(10\.10\.1\.2.*));if(/$re/){$1?print 
+$2:print qq(#$2)}"
     %WINDIR%\Sysnative\drivers\etc\hosts
[download]

L*