What slapped me in the face the instant I saw your script is:

• No subroutines!
• Poor scoping of variables: $ip_line, for example, is a glaring example of an evil global variable.

While this style of programming may work for small throw away scripts, it does not scale over time. As your program suite grows over time, you'll want to put utility subroutines into modules, along with unit tests, so the code can be understood in isolation, tested in isolation, and reused by multiple scripts. For that to work, you must avoid global variables. Instead, each subroutine must have well-defined inputs and outputs.

To illustrate, here's a sample subroutine to read your input file along with an illustrative trivial main program that calls it:

use strict;
use warnings;
use Data::Dumper;

# Read an input file
# Return a reference to a hash of properties
sub get_properties
{
   my $fname = shift;
   open my $fh, '<', $fname or die "open '$fname': $!";
   my %hash_ret;
   my $line;
   while ( defined( $line = <$fh> ) ) {
      chomp $line;
      $line =~ s/^\s+//;           # remove leading
      $line =~ s/\s+$//;           # and trailing whitespace
      next unless length $line;    # ignore empty lines
      next if $line =~ /^#/;       # ignore comment lines
      my ($key, $val) = split /\s*=\s*/, $line, 2;
      $hash_ret{$key} = $val;
   }
   close $fh;
   return \%hash_ret;
}

@ARGV or die "usage: $0 property-file\n";
my $prop_file = shift;
print "prop_file='$prop_file'\n";
my $properties = get_properties($prop_file);
print Dumper( $properties );
[download]

Note that this subroutine does not use any global variables, just reads the specified input file and returns a reference to a hash of properties ... so could be easily put into a module and reused by many different main programs in your suite.

A couple of related points from On Coding Standards and Code Reviews:

• Minimize the scope of variables, pragmas, etc.
• Think about how to test your code from the beginning. This improves code quality because: writing a test first forces you to focus on interface (from the point of view of the user); hard to test code is often hard to use; simpler interfaces are easier to test; functions that are encapsulated and easy to test are easy to reuse; components that are easy to mock are usually more flexible/extensible; testing components in isolation ensures they can be understood in isolation and promotes low coupling/high cohesion. See also: Effective Automated Testing and Test::More.

G'day Dazz,

"Readability and maintainability are more important to me that speed or brevity."

That's great. Here's some suggestions:

• Avoid experimental features: they are subject to change. Some weeks ago I wrote "Re: Modern exposition of "case" or Switch in perl 5.34?". There is discussion about switch with several examples of alternatives.
• I question the use of Tie::File. Is there a specific reason you chose that? I would have thought a simple open would have been clearer: it's more readable and commonly understood (I suspect many would need to consult the documentation to understand the Tie::File syntax).
• Consider the autodie pragma. Hand-crafting I/O exception messages is tedious and error-prone. Write less code and let Perl do the work for you.
• On comments in your code:
  • In general, only use comments to clarify; not to reiterate or state the obvious.
  • Many are completely superfluous; for instance, "# Declare constants" immediately before "use constant ..." statements.
  • Some may become incorrect as the code matures; for instance, you have "mb_ip.cfg" in at least two comments, if those comments were necessary, which I question, use "$ip_filename" (if the filename changes, you won't have to search for and edit every occurrence of the hard-coded filename).
  • Some are just wrong to start with; for example, "# A hash ... in order ..." — hashes are unordered.
• On constants:
  • It is customary to use uppercase: e.g. TRUE and FALSE; not true and false. Keeping to expected conventions will improve readability.
  • Group your constants into one or more blocks; for example,

    use constant {
        TRUE  => 1,
        FALSE => 0,
    };
    [download]

  • Note that my ($x, $y, $z); declares three variables all of which will evaluate to FALSE; there is no need to individually assign FALSE to each.
• A final else (or default or whatever you use) should not contain a condition; that would be elsif.
• When specifying whitespace in a regex, do so explicitly. Instead of "/^.*static\s*ip_address=/", it would better to write /^\s+static\sip_address=/; that matches leading whitespace (tabs and spaces) and won't match "whateverstaticip_address=":

  $ perl -E 'my $x = "whateverstaticip_address=X"; say $x =~ /^.*static\
  +s*ip_address=/ ? "YES" : "NO"'
  YES
  
  $ perl -E 'my $x = "whateverstaticip_address=X"; say $x =~ /^\s+static
  +\sip_address=/ ? "YES" : "NO"'
  NO
  [download]

• Use consistent indentation. That will greatly improve readability and facilitate easier and less error-prone maintenance.

There's quite a few points. There may be others that I didn't pick up on my first pass through your code.

Readability more important than brevity :( Well, I can try...

#!/usr/bin/perl

use strict; https://perlmonks.org/?node_id=11136353
use warnings;
use Path::Tiny;

my $dhcpcdfile = 'fake.353'; # FIXME filename
-w $dhcpcdfile or die "cannot write $dhcpcdfile";

my %ip_params = (
  interface      => "usb0",
  ip_address     => "1.1.1.0",
  routers        => "127.0.0.0",
  domain_servers => "1.1.1.1"
  );

%ip_params = ( %ip_params, # add new data to defaults
  path('inputfile.353')->slurp =~ /^(\w+)=(.*?)\s*$/gm ); # FIXME file
+name
use Data::Dump 'dd'; dd 'ip_params', \%ip_params;

{ # block scope for locals
local @ARGV = $dhcpcdfile;
local $^I = '.bak'; # make backup, do inplace edit
my $foundinterface = 0;
while( <> )
  {
  if( /^\s*profile\s+static_$ip_params{interface}\b.*\n/m ) # alter th
+is section
    {
    $foundinterface = 1;
    warn "found section for $ip_params{interface} at line $.\n";
    }
  elsif( $foundinterface and /static/ )
    {
    s/^\s*static\s+(\w+)=\K.*/$ip_params{$1}/m or warn "failed to chan
+ge $1";
    warn "set $1 to $ip_params{$1}\n";
    }
  elsif( $foundinterface )
    {
    $foundinterface = 0;
    warn "ending changes at line $.\n";
    }
  print;
  }
}
[download]

The warns and Data::Dump can be removed, of course, making it even more readable :)

Hi
I have had a go a re-crafting the code by @tybalt89 and other comments. It works but I don't know why. It also spits out errors.

The errors indicate $1 for the params hash is not initialised on the line:
 s/^\s*static\s+(\w+)=\K.*/$ip_params{$1}/m or warn "failed to change $1";
I can't see where it is, or should be, initialised.

If you think I should be able to work this out for myself, you are probably right but my excuse is that I am suffering the effects of concussion from a bicycle crash that nearly killed me in February. Writing some software is exercise for my brain in an attempt to restore something like normal service. As part of my mental gymnastics, I have also been refurbishing a vintage RF signal generator. The diary writeup is here: https://www.eevblog.com/forum/repair/wavetek-2025a-0-2-2-200mghz-rf-sig-gen-repair/
As with software, I haven't professionally worked on hardware for decades.

The current code looks like this:

#!/usr/bin/perl

##### FUNCTION
# To read ip settings from a machine generated file and
# write those ip settings to the dhcpcd.conf file.
#  This script is designed to be started by a systemd service.path
#  The service monitors the mb_ip.cfg file for change.  When the file 
+is changed
#  the service calls this script.
#  Updated ip settings are applied after reboot.

##### INPUT
#  Takes user updates of IP settings saved in to the file /home/mimir/
+mb_ip.cfg
#  The user setting are range checked by the app before being saved to
+ file.
#  The file mb_ip.cfg will only have ip settings that are valid (but n
+ot necessarily correct).

##### OUTPUT
#  Writes the ip settings to the /etc/dhcpcd.conf file.
#

#  Reference: https://www.perlmonks.org/?node_id=11136353
#
#  Dazz
#  ver 1.3
#  8 Sept 2021

use strict; 
use warnings;
use Path::Tiny;
use Data::Dump;

###### Input file with ip settings
my $ip_filename = '/home/mimir/mb_ip.cfg';  # The ip info entered by t
+he user into the GUI

###### Output dhcp configuration file
# my $dhcpcdfile = '/etc/dhcpcd.conf';
my $dhcpcdfile = 'd.conf';             # TEST

######################################################################
+######################

use constant { 
  FALSE => 0;
  TRUE  => 1;
};

# Load the input parameters ip_params from the input file
my %ip_params;
%ip_params = ( %ip_params, # add new data to defaults, split with "="
  path($ip_filename)->slurp =~ /^(\w+)=(.*?)\s*$/gm );

# Print the contents of ip_params.                       #TEST
use Data::Dump 'dd'; dd 'ip_params', \%ip_params;        #TEST

# Convert 2 lines DNS to 1 line DNS.
$ip_params{domain_name_servers} = $ip_params{domain_name_server_1}." "
+.$ip_params{domain_name_server_2};
delete($ip_params{domain_name_server_1});
delete($ip_params{domain_name_server_2});

# Print to check the contents ip_params with combined DNS line.       
+                #TEST
use Data::Dump 'dd'; dd 'ip_params', \%ip_params;                     
+ #TEST

{ # block scope for locals
local @ARGV = $dhcpcdfile;
local $^I = '.bak'; # make backup, do inplace edit
my $foundinterface = FALSE;
while( <> )
  {
  if( /^\s*profile\s+static_$ip_params{interface}\b.*\n/m ) # look for
+ profile with matching interface name
    {                                                       # format m
+atches 'static profile_eth0'
    $foundinterface = TRUE;
    warn "found section for $ip_params{interface} at line $.\n";
    }
  elsif( $foundinterface and /static/ )
    {
warn "ip param key : $ip_params{$0}\n"; #TEST 
warn "ip param val : $ip_params{$1}\n"; #TEST ***ERROR:  $1 not initia
+ted. Lines 72 and 73***
    s/^\s*static\s+(\w+)=\K.*/$ip_params{$1}/m or warn "failed to chan
+ge $1";  # match string before \K  Return string after \K
    # ?? What increments $ip_params through the hash ??
    # ?? What stops over-writing of other later sections ?? eg. wlan0
    warn "set $1 to $ip_params{$1}\n";
    }
  elsif( $foundinterface )
    {
    $foundinterface = FALSE;
    warn "ending changes at line $.\n";
    }
  # Output line to dhcpcdfile
  print;
  }
}
[download]

The input file is this: (slightly different to the previous version)

#IP Configuration
#Mon Sep 06 14:29:34 NZST 2021
routers=192.168.9.91
interface=eth0
domain_name_server_2=8.8.4.9
domain_name_server_1=8.8.8.9
ip_address=192.168.10.9/24
[download]

The abbreviated output file to be updated is this:

#static domain_name_servers=192.168.1.1

# fallback to static profile on eth0
#interface eth0
#fallback static_eth0
 
### Static LAN IP profile for Mimirbox
profile static_eth0
 static ip_address=1.1.1.1/24
 static routers=1.1.1.1
 static domain_name_servers=1.1.1.1 2.2.2.2

# Access Point Static interface mimirbox wlan0
  interface wlan0
  static ip_address=192.130.2.20/24
[download]

If these 3 files are placed in the same directory, the code should run.

Dazz

domain_name_servers does not exist in your input file. There is domain_name_server_2 and domain_name_server_1 instead.
Just a name mismatch.

What do you think the value of $0 is in the line ?

warn "ip param key : $ip_params{$0}\n"; #TEST
[download]

After a quick look at your code: how about removing comments before processing?

I think that's a must because you do allow comments (in your examples), but then you do when ( /interface/ ) { which will match a commented-out interface line or anything really that mentions the word "interface".

What I usually do for removing comment lines is $line =~ s/#.*$//; next if $line =~ /^\s*$/;

Secondly, if your grammar will grow then you are better off using other solutions, which other Monks I am sure will mention.

bw, bliako

Hi
Thanks for your feedback.

The input file is machine generated so the content and format is well defined. The regex is loose.
The output config file could be hand edited and so the regex is a lot more restrictive. Comments are ignored.

I am hoping to be shown a better way of doing this.


Dazz

I started using Switch, then I read in CPAN "do not use if you can use given/when". Then when I used given/when, I ended up with warnings plus advice not to use given/when. Hmmmmmm. As an occasional perl user, that is a little frustrating rewriting the same section of code 3x to do exactly the same thing. I just need something that has case type functionality.

I can certainly understand your frustration. This was a very sad affair for the Perl community. Though adding smart match into Perl was premature, and had to be backed out, at least P5P learned a valuable lesson.

Not wanting to gloat (no, really) but this didn't affect me, at all, because I've never been a fan of switch. Stronger, I've almost never used Switch in over 20 years of coding in C++ and Perl and always queried its use during code reviews. Though it's a bit extreme to call Switch a code smell, cleaner alternatives, such as lookup-tables (hashes in Perl) and polymorphism (in OO languages, such as C++ and Java) should be preferred.

From Perl Best Practices I suggest you take a look at the Control Structures chapter, especially:

• 6.16 Value Switches - Use table look-up in preference to cascaded equality tests (item 78)
• 6.17 Tabular Ternaries - When producing a value, use tabular ternaries (item 79)

Though my advice in Perl is usually "just use a hash", as a last resort you could replace your switch with an if-elsif-elsif-else construct.

References Added Later

• Re: Modern exposition of "case" or Switch in perl 5.34? by kcott (2021)
• Modern exposition of "case" or Switch in perl 5.34? by symgryph (2021)
• Hash versus chain of elsifs by mldvx4 (2021)
• 'switch' and 'smartmatch' features deprecated in 5.38 by ibm1620 (2023)
• Perl named operators and builtin subs introduced since 5.6 by ikegami (2023)
• Re: using smartmatch for range matching by ikegami (2023) - the unfortunate smartmatch chaos resulted in the creation of policies to make new features experimental
• using smartmatch for range matching by perltux (2023) - is smartmatch really being deprecated? (yes, it will be gone in perl 5.42)
• Smartmatch is deprecated (perldiag)
• Switch depreciated? by ksublondie (2016) - Perl::Critic warns that Switch is deprecated
• Switch statements are bad? (StackOverflow)
• Should I use switch statements or long if else chains? (StackExchange)
• Endorsement from committer to submit TPF Grant Application to add given/when to Switch.pm by ait (Apr 1 2024) - oops, hippo notes it's already there ... April Fools?

On CPAN:

• Switch by Alexandr Ciornii (chorny?) - a switch statement for Perl, do not use if you can use given/when
• Switch::Plain by Lukas Mai - a simple switch statement for Perl (not a source filter, uses pluggable keywords)
• Syntax::Keyword::Match by Paul Evans - a match/case syntax for perl
• XS::Parse::Keyword by Paul Evans - XS functions to assist in parsing keyword syntax

There is a very small, but finite risk of the Pi being turned off in the middle of a file write

Ha ha, I have long experience contemplating this annoying and tricky problem! The straightforward solution I concocted back in 2003 (and am still happy with) is to simply write a new file on the same file system ... and then use (atomic) rename to clobber the original file - but only after the new file has been successfully written. This is described in detail at:
• Re-runnably editing a file in place (see also its References Added Later section - especially CPAN module File::Replace by haukex)

See ->spew (and ->edit and ->edit_lines) in Path::Tiny

Solution using Path::Tiny which writes the new file alongside the existing file, then does an atomic rename. See Re^2: Yet another config file editing programme : Tell me how to make it better ! and Re^3: Yet another config file editing programme : Tell me how to make it better !

#!/usr/bin/perl

use strict; # https://perlmonks.org/?node_id=11136353
use warnings;
use Path::Tiny;

my $dhcpcdfile = 'fake.353'; # FIXME filename
-w $dhcpcdfile or die "cannot write $dhcpcdfile";

my %ip_params = (
  interface      => "usb0",
  ip_address     => "1.1.1.0",
  routers        => "127.0.0.0",
  domain_servers => "1.1.1.1"
  );

%ip_params = ( %ip_params, # add new data to defaults
  path('inputfile.353')->slurp =~ /^(\w+)=(.*?)\s*$/gm ); # FIXME file
+name

my $foundinterface = 0;
path( $dhcpcdfile )->edit_lines( sub
  {
  if( /^\s*profile\s+static_$ip_params{interface}\b.*\n/m ) # alter th
+is section
    {
    $foundinterface = 1;
    }
  elsif( $foundinterface and /^\s*static/m )
    {
    s/^\s*static\s+(\w+)=\K.*/$ip_params{$1}/m or die "failed to chang
+e $1";
    }
  elsif( $foundinterface )
    {
    $foundinterface = 0;
    }
  } );
[download]

I started using Switch, then I read in CPAN "do not use if you can use given/when". Then when I used given/when, I ended up with warnings plus advice not to use given/when. Hmmmmmm. As an occasional perl user, that is a little frustrating rewriting the same section of code 3x to do exactly the same thing.

I completely agree and understand. The (mis-)management of the switch/case equivalent in Perl over the years has become something of a lesson in how not to do it. given/when never struck me as a good idea so I just avoided it but plenty succumbed. Similarly, I never went anywhere near smartmatch but only a minority were enticed into that one. FWIW the current guidance is here. It will probably change again.

Sometimes being behind the curve is a good thing. These days I usually write Perl which is something like 5 years behind the current release in terms of features. This gives enough lead-in to be able to ascertain which "new" features have turned out to be turkeys and should be avoided. Such an approach is not for everyone, of course, but it can have its advantages (not least of which is back-compatibility).

---

🦛

Readability and brevity - together again !

#!/usr/bin/perl

use strict; # https://perlmonks.org/?node_id=11136353
use warnings;
use Path::Tiny;

my $dhcpcdfile = 'fake.353'; # FIXME filename

my %ip_params = (                   # defaults
  interface      => "usb0",
  ip_address     => "1.1.1.0",
  routers        => "127.0.0.0",
  domain_servers => "1.1.1.1"
  );

%ip_params = ( %ip_params, # add new data to defaults
  path('inputfile.353')->slurp =~ /^(\w+)=(.*?)\s*$/gm ); # FIXME file
+name

{ # block for local
local $/ = ''; # paragraph mode
path( $dhcpcdfile )->edit_lines( sub
  {
  if( /^\s*profile\s+static_$ip_params{interface}\b.*\n/m ) # alter th
+is section
    {
    s/^\s*static\s+(\w+)=\K.*/$ip_params{$1}/gm;
    }
  } );
}
[download]

{ # block for local
local $/ = ''; # paragraph mode
path( $dhcpcdfile )->edit_lines( sub
  {
  if( /^\s*profile\s+static_$ip_params{interface}\b.*\n/m ) # alter th
+is section
    {
    s/^\s*static\s+(\w+)=\K.*/$ip_params{$1}/gm;
    }
  } );
}
[download]

To an experienced perl programmer with deep knowledge of the language, this might look readable but for an occasional unprofessional perl user like myself, it would probably take at least half an hour to figure out what it is doing.
If I came back to this sort of code in a couple of years to alter/reuse it, I'd be back to square one.
If I had a future application that required a different input, this sort of all-in-one read/write approach would be difficult for me to repurpose.
It seems that brevity and obfuscation in perl code are inseparable.

For me, readability and brevity would be discovering a module Unix::ConfigFile::DHCPCD, that included a method "UpdateInterfaceIP". I think that a valid metric for a modern language is the code I don't have to write.

Please note I am not criticizing the skill or helpfulness those that give up their own time to write replies to people like me. I don't want to sound ungrateful. This is my goto place to find expert advice on perl, but claiming code is "readable" on a website that has a section devoted to "Obfuscation" is not a good look.



Dazz