pf-tools/pf-tools: critic

parmelan-guest at users.alioth.debian.org parmelan-guest at users.alioth.debian.org
Thu Aug 7 16:31:31 UTC 2014


details:   http://hg.debian.org/hg/pf-tools/pf-tools/rev/fc33cd0e9c62
changeset: 1305:fc33cd0e9c62
user:      shad
date:      Thu Aug 07 18:31:27 2014 +0200
description:
critic

diffstat:

 lib/PFTools/Update.pm            |  46 +++++++++---------
 lib/PFTools/Update/ADDFILE.pm    |  66 +++++++++++++++------------
 lib/PFTools/Update/ADDLINK.pm    |  35 +++++++-------
 lib/PFTools/Update/ADDMOUNT.pm   |  60 ++++++++++++++----------
 lib/PFTools/Update/CREATEFILE.pm |  37 +++++++-------
 lib/PFTools/Update/Common.pm     |  96 ++++++++++++++++++++++-----------------
 lib/PFTools/Update/IGNORE.pm     |   8 +-
 lib/PFTools/Update/INSTALLPKG.pm |  36 ++++++++------
 lib/PFTools/Update/MKDIR.pm      |  32 +++++++------
 lib/PFTools/Update/PURGEPKG.pm   |  38 +++++++-------
 lib/PFTools/Update/REMOVEDIR.pm  |  23 +++++----
 lib/PFTools/Update/REMOVEFILE.pm |  23 +++++----
 12 files changed, 269 insertions(+), 231 deletions(-)

diffs (1252 lines):

diff -r 9034a7ecbb2d -r fc33cd0e9c62 lib/PFTools/Update.pm
--- a/lib/PFTools/Update.pm	Thu Aug 07 15:15:52 2014 +0200
+++ b/lib/PFTools/Update.pm	Thu Aug 07 18:31:27 2014 +0200
@@ -51,15 +51,15 @@
 # Each action's package is loaded at runtime with Module::Runtime
 #
 # Each new action must provide at least two exported function
-#   - Action_depends    : for building potentially implicit depend(s)
-#   - Action_exec       : the action itself
+#   - action_depends    : for building potentially implicit depend(s)
+#   - action_exec       : the action itself
 #
-# Action_depends is a VOID function with 3 parameters
+# action_depends is a VOID function with 3 parameters
 #   - $ref_section  : HASHREF where parsed section is stored
 #   - $dest         : parsed section name
 #   - $options      : HASHREF with update-config options
 #
-# Action_exec is a BOOLEAN function with 5 parameters
+# action_exec is a BOOLEAN function with 5 parameters
 #   - $ref_section  : HASHREF where parsed section is stored
 #   - $dest         : parsed section name
 #   - $options      : HASHREF with update-config options
@@ -70,15 +70,15 @@
 ###########################################
 # Global vars
 
-my $STARTTIME  = time();
+my $STARTTIME  = time;
 my $APT_UPDATE = 1;
 
-sub __Init_action_engine {
+sub __init_action_engine {
     my ($action) = @_;
 
     return unless $action;
 
-    my $module_name = "PFTools::Update::" . uc($action);
+    my $module_name = q{PFTools::Update::} . uc $action;
     my $module;
     eval { $module = use_module($module_name); };
     if ($EVAL_ERROR) {
@@ -93,14 +93,14 @@
 sub Get_depends_for_action ($$$$) {
     my ( $action, $ref_section, $dest, $options ) = @_;
 
-    __Init_action_engine($action);
+    __init_action_engine($action);
 
     # Checking parameter
-    unless ( ref($ref_section) eq 'HASH' ) {
+    if ( ref($ref_section) ne 'HASH' ) {
         carp q{ERROR: non-hashref $ref_section};
         return;
     }
-    unless ( ref($options) eq 'HASH' ) {
+    if ( ref($options) ne 'HASH' ) {
         carp q{ERROR: non-hashref $options};
         return;
     }
@@ -109,7 +109,7 @@
         return;
     }
 
-    Action_depends( $ref_section, $dest, $options );
+    action_depends( $ref_section, $dest, $options );
     return;
 }
 
@@ -117,11 +117,11 @@
     my ( $action, $ref_section, $dest, $options, $hash_subst, $global_config )
         = @_;
 
-    __Init_action_engine($action);
+    __init_action_engine($action);
 
     # Adding some commons entries into substitution hash : $hash_subst
     $hash_subst->{'SECTIONNAME'} = $dest;
-    if ( $action eq "apt-get" || $action eq "installpkg" ) {
+    if ( $action eq q{apt-get} || $action eq q{installpkg} ) {
         if ($APT_UPDATE) {
             if ( !Update_pkg_repository( $options->{'pkg_type'} ) ) {
                 carp q{An error occured during updating packages lists};
@@ -130,7 +130,7 @@
             $APT_UPDATE = 0;
         }
     }
-    return Action_exec( $ref_section, $dest, $options, $hash_subst,
+    return action_exec( $ref_section, $dest, $options, $hash_subst,
         $global_config );
 }
 
@@ -140,13 +140,13 @@
     my $prio = 0;
 
     # First : authentication parts
-    return $prio if ( $section eq "/etc/passwd" );
+    return $prio if ( $section eq q{/etc/passwd} );
     $prio++;
-    return $prio if ( $section eq "/etc/group" );
+    return $prio if ( $section eq q{/etc/group} );
     $prio++;
-    return $prio if ( $section eq "/etc/shadow" );
+    return $prio if ( $section eq q{/etc/shadow} );
     $prio++;
-    return $prio if ( $section eq "/etc/gshadow" );
+    return $prio if ( $section eq q{/etc/gshadow} );
     $prio++;
 
     # Second : directory and mount points
@@ -156,13 +156,13 @@
     $prio++;
 
     # Third : Packaging infra and packages
-    return $prio if ( $section =~ /^\/etc\/apt\// );
+    return $prio if ( $section =~ m{\A /etc/apt/ }xms );
     $prio++;
-    return $prio if ( $section eq "pf-tools" );
+    return $prio if ( $section eq q{pf-tools} );
     $prio++;
-    return $prio if ( $action eq "dpkg-purge" || $action eq "purgepkg" );
+    return $prio if ( $action eq q{dpkg-purge} || $action eq q{purgepkg} );
     $prio++;
-    return $prio if ( $action eq "apt-get" || $action eq "installpkg" );
+    return $prio if ( $action eq q{apt-get} || $action eq q{installpkg} );
     $prio++;
 
     # Fourth : creations and adds for files and links
@@ -174,7 +174,7 @@
     $prio++;
 
     # Fifth : removing files and dirs
-    return $prio if ( $action =~ /^remove/ );
+    return $prio if ( $action =~ /\A remove/xms );
     $prio++;
 
     # Last : other elements
diff -r 9034a7ecbb2d -r fc33cd0e9c62 lib/PFTools/Update/ADDFILE.pm
--- a/lib/PFTools/Update/ADDFILE.pm	Thu Aug 07 15:15:52 2014 +0200
+++ b/lib/PFTools/Update/ADDFILE.pm	Thu Aug 07 18:31:27 2014 +0200
@@ -34,46 +34,50 @@
 use PFTools::Update::Common;
 
 our @EXPORT = qw(
-    Action_depends
-    Action_exec
+    action_depends
+    action_exec
 );
 
 our @EXPORT_OK = qw();
 
-sub Action_depends {
+sub action_depends {
     my ( $ref_section, $dest, $options ) = @_;
 
-    while ( $dest ne "/" && $dest ne "." ) {
+    while ( $dest ne q{/} && $dest ne q{.} ) {
         my $new_dest = dirname($dest);
-        $ref_section->{'depends'} .= " " . $new_dest
-            if ( $new_dest ne "." && $new_dest ne "/" );
+        if ( $new_dest ne q{.} && $new_dest ne q{/} ) {
+            $ref_section->{'depends'} .= q{ } . $new_dest;
+        }
         $dest = $new_dest;
     }
+    return;
 }
 
-sub Action_exec {
+sub action_exec {
     my ( $ref_section, $dest, $options, $hash_subst, $global_config ) = @_;
-    my ( $source, $tmp, $cmp );
+    my ( $source, $tmp );
 
     my $diff = 0;
     $hash_subst->{'SECTIONNAME'} = $dest;
-    $tmp = Get_tmp_dest($dest);
+    $tmp = get_tmp_dest($dest);
 
     # Removing trailing space from source
-    $ref_section->{'source'} =~ s{\A\s*}{};
-    $ref_section->{'source'} =~ s{\s*\z}{};
-    if ( $ref_section->{'source'} =~ m{\s} ) {
-        $source = Get_tmp_dest($dest) . q{.merged};
-        unlink($source);
-        foreach my $splitsource ( split( ' ', $ref_section->{'source'} ) ) {
+    $ref_section->{'source'} =~ s{\A\s*}{}xms;
+    $ref_section->{'source'} =~ s{\s*\z}{}xms;
+    if ( $ref_section->{'source'} =~ m{\s}xms ) {
+        $source = get_tmp_dest($dest) . q{.merged};
+        unlink $source;
+        foreach my $splitsource ( split q{ }, $ref_section->{'source'} ) {
             $splitsource
                 = Get_source( $splitsource, $options->{'host'}, $hash_subst );
             if ( !-f $splitsource ) {
-                carp colored( qq{ERROR: $splitsource no such file or directory}, q{bold red on_white} );
+                carp colored(
+                    qq{ERROR: $splitsource no such file or directory},
+                    q{bold red on_white} );
                 return;
             }
             if (deferredlogsystem(
-                    "cat '" . $splitsource . "' >> " . $source
+                    q{cat '} . $splitsource . q{' >> } . $source
                 )
                 )
             {
@@ -88,7 +92,8 @@
     }
 
     if ( !-e $source ) {
-        carp colored( qq{ERROR: $source no such file or directory}, q{bold red on_white} );
+        carp colored( qq{ERROR: $source no such file or directory},
+            q{bold red on_white} );
         return;
     }
     $hash_subst->{'SOURCE'}      = $source;
@@ -101,7 +106,7 @@
         }
     }
     else {
-        unless ( copy( $source, $tmp ) ) {
+        if ( !copy( $source, $tmp ) ) {
             carp qq{ERROR: Unable to copy $source to $tmp};
             return;
         }
@@ -120,41 +125,42 @@
         if ( $options->{'diff'} ) {
             my @diff;
             if ( !-e $dest ) {
-                @diff = split qq{\n},
+                @diff = split /\n/xms,
                     diff( [], $tmp, { STYLE => 'Unified' } );
                 print_diff_color(@diff);
             }
             else {
-                @diff = split qq{\n},
+                @diff = split /\n/xms,
                     diff( $dest, $tmp, { STYLE => 'Unified' } );
                 print_diff_color(@diff);
             }
         }
         print "on_config ...\n";
-        Do_on_config( $ref_section, $options, $hash_subst ) or return;
+        do_on_config( $ref_section, $options, $hash_subst ) or return;
         print "before_change ...\n";
-        Do_before_change( $ref_section, $options, $hash_subst ) or return;
+        do_before_change( $ref_section, $options, $hash_subst ) or return;
         if ( !$options->{'simul'} ) {
 
             # Fuck dpkg conffiles
             if (   $options->{'noaction'}
                 && -e $dest
-                && !-e $dest . '.dpkg-dist' )
+                && !-e qq{${dest}.dpkg-dist} )
             {
                 copy( $dest, $dest . '.dpkg-dist' );
             }
-            Do_moveold( $dest, $options );
-            if ( !Mk_dest_dir($dest) || !copy( $tmp, $dest ) ) {
-                carp colored( qq{ERROR: Unable to copy file $tmp to $dest}, q{bold red on_white} );
+            do_moveold( $dest, $options );
+            if ( !mk_dest_dir($dest) || !copy( $tmp, $dest ) ) {
+                carp colored( qq{ERROR: Unable to copy file $tmp to $dest},
+                    q{bold red on_white} );
                 return;
             }
-            Do_chownmod( $ref_section, $dest, $options );
+            do_chownmod( $ref_section, $dest, $options );
         }
         if ($diff) {
             print "after_change ...\n";
-            Do_after_change( $ref_section, $options, $hash_subst ) or return;
+            do_after_change( $ref_section, $options, $hash_subst ) or return;
             print "on_noaction ...\n";
-            Do_on_noaction( $ref_section, $options, $hash_subst ) or return;
+            do_on_noaction( $ref_section, $options, $hash_subst ) or return;
         }
     }
     return 1;
diff -r 9034a7ecbb2d -r fc33cd0e9c62 lib/PFTools/Update/ADDLINK.pm
--- a/lib/PFTools/Update/ADDLINK.pm	Thu Aug 07 15:15:52 2014 +0200
+++ b/lib/PFTools/Update/ADDLINK.pm	Thu Aug 07 18:31:27 2014 +0200
@@ -31,22 +31,23 @@
 use PFTools::Update::Common;
 
 our @EXPORT = qw(
-    Action_depends
-    Action_exec
+    action_depends
+    action_exec
 );
 
 our @EXPORT_OK = qw();
 
-sub Action_depends {
+sub action_depends {
     my ( $ref_section, $dest, $options ) = @_;
 
-    while ( $dest ne "/" && $dest ne "." ) {
-        $ref_section->{'depends'} .= " " . dirname($dest);
+    while ( $dest ne q{/} && $dest ne q{.} ) {
+        $ref_section->{'depends'} .= q{ } . dirname($dest);
         $dest = dirname($dest);
     }
+    return;
 }
 
-sub Action_exec {
+sub action_exec {
     my ( $ref_section, $dest, $options, $hash_subst, $global_config ) = @_;
 
     $hash_subst->{'SECTIONNAME'} = $dest;
@@ -54,33 +55,33 @@
 
     # Need to check the source ...
     my $dep_src = $source;
-    while ( $dep_src ne "/" && $dep_src ne "." ) {
-        $ref_section->{'depends'} .= " " . dirname($dep_src);
+    while ( $dep_src ne q{/} && $dep_src ne q{.} ) {
+        $ref_section->{'depends'} .= q{ } . dirname($dep_src);
         $dep_src = dirname($dep_src);
     }
     if ( !-l $dest || ( -l $dest && readlink($dest) ne $source ) ) {
         if ( $options->{'verbose'} || $options->{'simul'} ) {
-            Log(colored(q{(action needed)}, q{bold red}));
+            Log( colored( q{(action needed)}, q{bold red} ) );
         }
         if ( $options->{'diff'} ) {
             if ( -l $dest ) {
-                Log( "( readlink = " . readlink($dest) . ")" );
+                Log( q{( readlink = } . readlink($dest) . q{)} );
             }
             else {
-                Log( "( ! -l " . $dest . ")" );
+                Log( q{( ! -l } . $dest . q{)} );
             }
         }
-        Do_on_config( $ref_section, $options, $hash_subst ) or return;
-        Do_before_change( $ref_section, $options, $hash_subst ) or return;
+        do_on_config( $ref_section, $options, $hash_subst ) or return;
+        do_before_change( $ref_section, $options, $hash_subst ) or return;
         if ( !$options->{'simul'} ) {
-            Do_moveold( $dest, $options );
-            if ( !Mk_dest_dir($dest) || !ln_sfn( $source, $dest ) ) {
+            do_moveold( $dest, $options );
+            if ( !mk_dest_dir($dest) || !ln_sfn( $source, $dest ) ) {
                 carp qq{ERROR: Unable to symlink $dest to $source};
                 return;
             }
         }
-        Do_after_change( $ref_section, $options, $hash_subst ) or return;
-        Do_on_noaction( $ref_section, $options, $hash_subst ) or return;
+        do_after_change( $ref_section, $options, $hash_subst ) or return;
+        do_on_noaction( $ref_section, $options, $hash_subst ) or return;
     }
     return 1;
 }
diff -r 9034a7ecbb2d -r fc33cd0e9c62 lib/PFTools/Update/ADDMOUNT.pm
--- a/lib/PFTools/Update/ADDMOUNT.pm	Thu Aug 07 15:15:52 2014 +0200
+++ b/lib/PFTools/Update/ADDMOUNT.pm	Thu Aug 07 18:31:27 2014 +0200
@@ -41,8 +41,8 @@
 #use PFTools::Update::Mkdir;
 
 our @EXPORT = qw(
-    Action_depends
-    Action_exec
+    action_depends
+    action_exec
 );
 
 #    Addmount_depends
@@ -59,24 +59,35 @@
 ###############################################
 # Functions
 
-sub Action_depends {
+sub action_depends {
     my ( $ref_section, $dest, $options ) = @_;
 
     while ( $dest ne q{/} && $dest ne q{.} ) {
         $ref_section->{'depends'} .= q{ } . dirname($dest);
         $dest = dirname($dest);
     }
+    return;
 }
 
-sub __Get_ip_host_from_GLOBAL {
+sub __get_ip_host_from_global {
     my ( $host, $global_config ) = @_;
 
+    my ( $zone, $hostshort, $hostvlan );
     my $ip = $host;
-    $host =~ m{\A ([^\.]+)([.].*)? \z}xms;
-    my $zone = get_zone_from_hostname( $1, $global_config );
+    if ( $host =~ m{\A ( [^.]+ ) (?: [.].* )? \z}xms ) {
+        $zone = get_zone_from_hostname( $1, $global_config );
+    }
+    else {
+        carp qq{ERROR: no host found for $host};
+        return;
+    }
     $ip =~ s{[.]$zone \z}{}xms;
-    $ip =~ m{\A ([^.]+)([.]([^.]+))? \z}xms;
-    my ( $hostshort, $hostvlan ) = ( $1, $3 );
+    if ( $ip =~ m{\A ( [^.]+ ) (?: [.]([^.]+) )? \z}xms ) {
+        ( $hostshort, $hostvlan ) = ( $1, $3 );
+    }
+    else {
+        carp qq{ERROR: can't extract hostshort and hostvlan from $ip};
+    }
     my $hosttype = get_hosttype_from_hostname( $hostshort, $global_config );
     my $site_list = get_site_list_from_hostname( $hostshort, $global_config );
     my $site;
@@ -110,7 +121,7 @@
     return $ip;
 }
 
-sub __Resolve_fstab_entry {
+sub __resolve_fstab_entry {
     my ($param) = @_;
 
     my $pf_config = Init_PF_CONFIG();
@@ -131,7 +142,7 @@
             $val_addr =~ s{$regex}{$+{ip}};
 
             if ( defined $val_addr && $val_addr ne $value ) {
-                my $val_ip = __Get_ip_host_from_GLOBAL( $val_addr,
+                my $val_ip = __get_ip_host_from_global( $val_addr,
                     $param->{'global_config'} );
                 return if ( !$val_ip );
                 $regex
@@ -146,7 +157,7 @@
     return 1;
 }
 
-sub __Build_fstab_entry_from_config {
+sub __build_fstab_entry_from_config {
     my ($param) = @_;
 
     my $fs_entry = $param->{'ref_section'};
@@ -157,25 +168,25 @@
     }
 
     $fs_entry->{'options'}
-        = join( q{,}, sort split /,/, $fs_entry->{'options'} );
+        = join( q{,}, sort split /,/xms, $fs_entry->{'options'} );
 
     my $resolve_param = {
         'fs_entry'      => $fs_entry,
         'global_config' => $param->{'global_config'}
     };
-    if ( !__Resolve_fstab_entry($resolve_param) ) {
+    if ( !__resolve_fstab_entry($resolve_param) ) {
         return;
     }
     return $fs_entry;
 }
 
-sub Action_exec {
+sub action_exec {
     my ( $ref_section, $dest, $options, $hash_subst, $global_config ) = @_;
 
     $hash_subst->{'SECTIONNAME'} = $dest;
 
     # Source
-    my $add_mount = __Build_fstab_entry_from_config(
+    my $add_mount = __build_fstab_entry_from_config(
         {   'dest'          => $dest,
             'subst'         => $hash_subst,
             'global_config' => $global_config,
@@ -188,7 +199,7 @@
     }
     $hash_subst->{'SOURCE'} = $add_mount->{'source'};
     $hash_subst->{'OPTIONS'}
-        = join( q{,}, sort split( /,/, $add_mount->{'options'} ) );
+        = join( q{,}, sort split( /,/xms, $add_mount->{'options'} ) );
     $hash_subst->{'FSTYPE'} = $ref_section->{'fstype'} || $DEFAULT_FSTYPE;
 
     my $current_fstab = Build_structure_from_fstab(q{/etc/fstab});
@@ -244,8 +255,8 @@
         if ( $options->{'verbose'} || $options->{'simul'} ) {
             Log( colored( q{(action needed)}, q{bold red} ) );
         }
-        Do_on_config( $ref_section, $options, $hash_subst ) or return;
-        Do_before_change( $ref_section, $options, $hash_subst ) or return;
+        do_on_config( $ref_section, $options, $hash_subst ) or return;
+        do_before_change( $ref_section, $options, $hash_subst ) or return;
         if ( !-d $dest && $dest ne 'none' ) {
             if ( !make_path($dest) ) {
                 carp qq{ERROR: while creating mountpoints $dest};
@@ -254,7 +265,7 @@
         }
         if ($addfstab) {
             my $fstabfile = q{/etc/fstab};
-            my $tmp       = Get_tmp_dest($fstabfile);
+            my $tmp       = get_tmp_dest($fstabfile);
             my $output_fh;
             my @diff;
 
@@ -275,7 +286,7 @@
             if ( $options->{'diff'} ) {
 
                 #print diff ( $fstabfile, $tmp, { STYLE => 'Unified' } );
-                @diff = split qq{\n},
+                @diff = split /\n/xms,
                     diff( $fstabfile, $tmp, { STYLE => q{Unified} } );
                 print_diff_color(@diff);
             }
@@ -325,9 +336,8 @@
                         . $add_mount->{'options'} . q{' '}
                         . $dest . q{'};
                     if ( deferredlogsystem($cmd) ) {
-                        carp qq{ERROR: while remounting $dest with
-                            $add_mount->{'options'}
-                        };
+                        carp
+                            qq{ERROR: while remounting $dest with\n$add_mount->{'options'}};
                         return;
                     }
                 }
@@ -356,8 +366,8 @@
                 }
             }
         }
-        Do_after_change( $ref_section, $options, $hash_subst ) or return;
-        Do_on_noaction( $ref_section, $options, $hash_subst ) or return;
+        do_after_change( $ref_section, $options, $hash_subst ) or return;
+        do_on_noaction( $ref_section, $options, $hash_subst ) or return;
     }
     return 1;
 }
diff -r 9034a7ecbb2d -r fc33cd0e9c62 lib/PFTools/Update/CREATEFILE.pm
--- a/lib/PFTools/Update/CREATEFILE.pm	Thu Aug 07 15:15:52 2014 +0200
+++ b/lib/PFTools/Update/CREATEFILE.pm	Thu Aug 07 18:31:27 2014 +0200
@@ -32,30 +32,31 @@
 use PFTools::Update::Common;
 
 our @EXPORT = qw(
-    Action_depends
-    Action_exec
+    action_depends
+    action_exec
 );
 
 our @EXPORT_OK = qw();
 
-sub Action_depends {
+sub action_depends {
     my ( $ref_section, $dest, $options ) = @_;
 
-    while ( $dest ne "/" && $dest ne "." ) {
-        $ref_section->{'depends'} .= " " . dirname($dest);
+    while ( $dest ne q{/} && $dest ne q{.} ) {
+        $ref_section->{'depends'} .= q{ } . dirname($dest);
         $dest = dirname($dest);
     }
+    return;
 }
 
-sub Action_exec {
+sub action_exec {
     my ( $ref_section, $dest, $options, $hash_subst, $global_config ) = @_;
 
     $hash_subst->{'SECTIONNAME'} = $dest;
-    my $tmp = Get_tmp_dest($dest);
+    my $tmp = get_tmp_dest($dest);
     $hash_subst->{'DESTINATION'} = $tmp;
-    unless ( -f $dest ) {
+    if ( !-f $dest ) {
         if ( $options->{'verbose'} || $options->{'simul'} ) {
-            Log(colored(q{(action needed)}, q{bold red}));
+            Log( colored( q{(action needed)}, q{bold red} ) );
         }
         if ( !defined $ref_section->{'source'} ) {
 
@@ -66,13 +67,11 @@
             }
         }
         else {
-            my $source = Get_source(
-                $ref_section->{'source'}, $hash_subst->{'HOSTNAME'},
-                $hash_subst
-            );
+            my $source = Get_source( $ref_section->{'source'},
+                $hash_subst->{'HOSTNAME'}, $hash_subst );
 
             # Creating tmp destination from source
-            unless ( -f $source ) {
+            if ( !-f $source ) {
                 carp qq{ERROR: Unable to open source $source};
                 return;
             }
@@ -91,15 +90,15 @@
                 }
             }
         }
-        Do_on_config( $ref_section, $options, $hash_subst ) or return;
-        Do_before_change( $ref_section, $options, $hash_subst ) or return;
+        do_on_config( $ref_section, $options, $hash_subst ) or return;
+        do_before_change( $ref_section, $options, $hash_subst ) or return;
         if ( !$options->{'simul'} && !copy( $tmp, $dest ) ) {
             carp qq{ERROR: Unable to create $dest with $tmp};
             return;
         }
-        Do_chownmod( $ref_section, $dest, $options ) or return;
-        Do_after_change( $ref_section, $options, $hash_subst ) or return;
-        Do_on_noaction( $ref_section, $options, $hash_subst ) or return;
+        do_chownmod( $ref_section, $dest, $options ) or return;
+        do_after_change( $ref_section, $options, $hash_subst ) or return;
+        do_on_noaction( $ref_section, $options, $hash_subst ) or return;
     }
     return 1;
 }
diff -r 9034a7ecbb2d -r fc33cd0e9c62 lib/PFTools/Update/Common.pm
--- a/lib/PFTools/Update/Common.pm	Thu Aug 07 15:15:52 2014 +0200
+++ b/lib/PFTools/Update/Common.pm	Thu Aug 07 18:31:27 2014 +0200
@@ -32,15 +32,15 @@
 use PFTools::Logger;
 
 our @EXPORT = qw(
-    Do_on_config
-    Do_on_noaction
-    Do_before_change
-    Do_after_change
-    Do_chownmod
-    Do_moveold
-    Exec_cmd
-    Mk_dest_dir
-    Get_tmp_dest
+    do_on_config
+    do_on_noaction
+    do_before_change
+    do_after_change
+    do_chownmod
+    do_moveold
+    exec_cmd
+    mk_dest_dir
+    get_tmp_dest
     dirname
     isipaddr
     ln_sfn
@@ -98,20 +98,24 @@
         if (   ( $uid && $uid == $newuid )
             && ( $gid && $gid == $newgid ) )
         {
-            print qq{Useless $type for $dest\n} if ( $options->{'verbose'} );
+            if ( $options->{'verbose'} ) {
+                print qq{Useless $type for $dest\n};
+            }
             return 1;
         }
         return chown $newuid, $newgid, $dest;
     }
     elsif ( $type eq 'chmod' ) {
         if ( $mode && ( $mode & oct 7777 ) == $right1 ) {
-            print qq{Useless $type for $dest\n} if ( $options->{'verbose'} );
+            if ( $options->{'verbose'} ) {
+                print qq{Useless $type for $dest\n};
+            }
             return 1;
         }
         return chmod $right1, $dest;
     }
     else {
-        carp q{ERROR: invalid $type parameter};
+        carp q{ERROR: invalid 'type' parameter};
         return;
     }
     return 1;
@@ -144,7 +148,9 @@
 
     if ( $file =~ m{/}xms ) {
         $file =~ s{\A (.*)/[^/]+/? \z}{$1}xms;
-        $file = q{.} if ( $file =~ m{\A \s* \z}xms );
+        if ( $file =~ m{\A \s* \z}xms ) {
+            $file = q{.};
+        }
     }
     else {
         $file = q{.};
@@ -153,7 +159,7 @@
     return $file;
 }
 
-sub Do_moveold {
+sub do_moveold {
     my ( $dest, $options ) = @_;
 
     my $pf_config = Init_PF_CONFIG();
@@ -163,22 +169,24 @@
             . q{/old/}
             . $dest . q{.}
             . $STARTTIME;
-        Log(qq{(moving old to $old)}) if ( $options->{'verbose'} );
-        unless ( $options->{'simul'} ) {
-            Mk_dest_dir($old);
+        if ( $options->{'verbose'} ) {
+            Log(qq{(moving old to $old)});
+        }
+        if ( !$options->{'simul'} ) {
+            mk_dest_dir($old);
             return move( $dest, $old );
         }
     }
     return 1;
 }
 
-sub Do_chownmod {
+sub do_chownmod {
     my ( $ref_section, $dest, $options ) = @_;
 
     my $owner = $ref_section->{'owner'} || $DEFAULT_OWNER;
     my $group = $ref_section->{'group'} || $DEFAULT_GROUP;
 
-    unless ( fullchown( $owner, $group, $dest, $options ) ) {
+    if ( !fullchown( $owner, $group, $dest, $options ) ) {
         carp qq{ERROR: Unable to chown with $owner and/or $group for $dest};
         return;
     }
@@ -189,14 +197,14 @@
         :                                     $DEFAULT_MODE;
     $mode =~ s{\A [^0]}{0$&}xms;
 
-    unless ( fullchmod( eval($mode), $dest, $options ) ) {
+    if ( !fullchmod( eval($mode), $dest, $options ) ) {
         carp qq{ERROR: Unable to chmod to $mode for $dest};
         return;
     }
     return 1;
 }
 
-sub Exec_cmd {
+sub exec_cmd {
     my ($cmd) = @_;
 
     if ( deferredlogsystem($cmd) ) {
@@ -206,84 +214,88 @@
     return 1;
 }
 
-sub Do_on_config ($$$) {
+sub do_on_config {
     my ( $ref_section, $options, $hash_subst ) = @_;
 
     if ( $ref_section->{'actiongroup'} ) {
-        Log( q{Triggering actiongroup } . $ref_section->{'actiongroup'} )
-            if ( $options->{'verbose'} );
+        if ( $options->{'verbose'} ) {
+            Log( q{Triggering actiongroup } . $ref_section->{'actiongroup'} );
+        }
         return 1;
     }
     if ( !$options->{'simul'}
         && defined( $ref_section->{'on_config'} ) )
     {
-        return Exec_cmd(
+        return exec_cmd(
             Subst_vars( $ref_section->{'on_config'}, $hash_subst ) );
     }
     return 1;
 }
 
-sub Do_before_change ($$$) {
+sub do_before_change {
     my ( $ref_section, $options, $hash_subst ) = @_;
 
     if ( $ref_section->{'actiongroup'} ) {
-        Log( q{Triggering actiongroup } . $ref_section->{'actiongroup'} )
-            if ( $options->{'verbose'} );
+        if ( $options->{'verbose'} ) {
+            Log( q{Triggering actiongroup } . $ref_section->{'actiongroup'} );
+        }
         return 1;
     }
     if (   !$options->{'simul'}
         && !$options->{'noaction'}
         && defined( $ref_section->{'before_change'} ) )
     {
-        return Exec_cmd(
+        return exec_cmd(
             Subst_vars( $ref_section->{'before_change'}, $hash_subst ) );
     }
     return 1;
 }
 
-sub Do_after_change ($$$) {
+sub do_after_change {
     my ( $ref_section, $options, $hash_subst ) = @_;
 
     if ( $ref_section->{'actiongroup'} ) {
-        Log( q{Triggering actiongroup } . $ref_section->{'actiongroup'} )
-            if ( $options->{'verbose'} );
+        if ( $options->{'verbose'} ) {
+            Log( q{Triggering actiongroup } . $ref_section->{'actiongroup'} );
+        }
         return 1;
     }
     if (   !$options->{'simul'}
         && defined( $ref_section->{'after_change'} )
         && !$options->{'noaction'} )
     {
-        return Exec_cmd(
+        return exec_cmd(
             Subst_vars( $ref_section->{'after_change'}, $hash_subst ) );
     }
     return 1;
 }
 
-sub Do_on_noaction ($$$) {
+sub do_on_noaction {
     my ( $ref_section, $options, $hash_subst ) = @_;
 
     if ( $ref_section->{'actiongroup'} ) {
-        Log( q{Triggering actiongroup } . $ref_section->{'actiongroup'} )
-            if ( $options->{'verbose'} );
+        if ( $options->{'verbose'} ) {
+            Log( q{Triggering actiongroup } . $ref_section->{'actiongroup'} );
+        }
         return 1;
     }
     if (   !$options->{'simul'}
         && defined( $ref_section->{'on_noaction'} )
         && $options->{'noaction'} )
     {
-        return Exec_cmd(
+        return exec_cmd(
             Subst_vars( $ref_section->{'on_noaction'}, $hash_subst ) );
     }
     return 1;
 }
 
-# Mk_dest_dir: wrapper around make_path, trying to remove any existing
+# mk_dest_dir: wrapper around make_path, trying to remove any existing
 # non-directory that would prevent make_path to create the directory.
 # Returns true on success.
-sub Mk_dest_dir {
+sub mk_dest_dir {
     my ($dir) = @_;
 
-    return unless $dir;
+    return if !$dir;
 
     # FIXME rewrite this part, it's really unreadable!!
     $dir =~ s{//}{/}gxms;          # supprimer // sinon ca marche moins bien
@@ -305,12 +317,12 @@
     return 1;    # OK
 }
 
-sub Get_tmp_dest ($) {
+sub get_tmp_dest {
     my ($dest) = @_;
 
     my $pf_config = Init_PF_CONFIG();
     my $tmp       = $pf_config->{'path'}->{'checkout_dir'} . q{/tmp/} . $dest;
-    Mk_dest_dir($tmp);
+    mk_dest_dir($tmp);
     if ( -d $tmp ) {
         rmdir $tmp;
     }
diff -r 9034a7ecbb2d -r fc33cd0e9c62 lib/PFTools/Update/IGNORE.pm
--- a/lib/PFTools/Update/IGNORE.pm	Thu Aug 07 15:15:52 2014 +0200
+++ b/lib/PFTools/Update/IGNORE.pm	Thu Aug 07 18:31:27 2014 +0200
@@ -31,17 +31,17 @@
 use PFTools::Update::Common;
 
 our @EXPORT = qw(
-    Action_depends
-    Action_exec
+    action_depends
+    action_exec
 );
 
 our @EXPORT_OK = qw();
 
-sub Action_depends {
+sub action_depends {
     return 1;
 }
 
-sub Action_exec {
+sub action_exec {
     my ( $ref_section, $dest, $options, $hash_subst, $global_config ) = @_;
 
     if ( $options->{'verbose'} || $options->{'simul'} ) {
diff -r 9034a7ecbb2d -r fc33cd0e9c62 lib/PFTools/Update/INSTALLPKG.pm
--- a/lib/PFTools/Update/INSTALLPKG.pm	Thu Aug 07 15:15:52 2014 +0200
+++ b/lib/PFTools/Update/INSTALLPKG.pm	Thu Aug 07 18:31:27 2014 +0200
@@ -35,8 +35,8 @@
 use PFTools::Update::Common;
 
 our @EXPORT = qw(
-    Action_depends
-    Action_exec
+    action_depends
+    action_exec
 );
 
 our @EXPORT_OK = qw();
@@ -47,10 +47,12 @@
 $ENV{'DEBIAN_FRONTEND'} = 'noninteractive';
 $ENV{'DEBIAN_PRIORITY'} = 'critical';
 
-sub Action_depends {
+sub action_depends {
     my ( $ref_section, $dest, $options ) = @_;
 
-    $options->{'pkg_type'} = 'deb' unless ( $options->{'pkg_type'} );
+    if ( !$options->{'pkg_type'} ) {
+        $options->{'pkg_type'} = 'deb';
+    }
     my $deps = Get_pkg_depends( $options->{'pkg_type'}, $dest );
     if ( !$deps ) {
         if ( $options->{'verbose'} ) {
@@ -69,10 +71,12 @@
     return;
 }
 
-sub Action_exec {
+sub action_exec {
     my ( $ref_section, $dest, $options, $hash_subst, $global_config ) = @_;
 
-    $options->{'pkg_type'} = 'deb' unless ( $options->{'pkg_type'} );
+    if ( !$options->{'pkg_type'} ) {
+        $options->{'pkg_type'} = 'deb';
+    }
     my $installed_version;
     my $available_version;
     my $specified_version = 0;
@@ -82,11 +86,11 @@
     if ($name_filter) {
         my $newdest
             = deferredlogpipe( Subst_vars( $name_filter, $hash_subst ) );
-        unless ( defined $newdest ) {
+        if ( !defined $newdest ) {
             carp qq{ERROR: Unable to apply name_filter $name_filter};
             return;
         }
-        unless ($newdest) {
+        if ( !$newdest ) {
             carp qq{ERROR: Empty result for name_filter $name_filter};
             return;
         }
@@ -96,7 +100,7 @@
     ( $installed_version, $available_version, $specified_version )
         = Get_pkg_policy( $options->{'pkg_type'},
         $dest, $ref_section->{'version'} );
-    unless ($available_version) {
+    if ( !$available_version ) {
         carp qq{ERROR: Package $dest is unavailable};
         return;
     }
@@ -110,7 +114,9 @@
     else {
         my $compare = Cmp_pkg_version( $options->{'pkg_type'},
             $dest, $installed_version, $available_version );
-        $install++ if ( defined $compare && $compare < 0 );
+        if ( defined $compare && $compare < 0 ) {
+            $install++;
+        }
     }
     if ($install) {
         if ( $options->{'verbose'} || $options->{'simul'} ) {
@@ -144,7 +150,7 @@
             foreach my $key ( keys %{$ref_section} ) {
                 next if ( $key !~ m{\A debconf}xmso );
                 $debconf = 1;
-                $key =~ m{\A debconf\.(.*) \z}xmso;
+                $key =~ m{\A debconf[.](.*) \z}xmso;
                 $debconf_vars->{$1} = $ref_section->{$key};
             }
             if ($debconf) {
@@ -168,8 +174,8 @@
                 Debconf::Db->save;
             }
         }
-        Do_on_config( $ref_section, $options, $hash_subst ) or return;
-        Do_before_change( $ref_section, $options, $hash_subst ) or return;
+        do_on_config( $ref_section, $options, $hash_subst ) or return;
+        do_before_change( $ref_section, $options, $hash_subst ) or return;
         if ( defined( $ref_section->{'reply'} ) ) {
             $install = $ref_section->{'reply'};
             eval "\$install = sprintf (\"echo '$install' |\")";
@@ -188,8 +194,8 @@
                 return;
             }
         }
-        Do_after_change( $ref_section, $options, $hash_subst ) or return;
-        Do_on_noaction( $ref_section, $options, $hash_subst ) or return;
+        do_after_change( $ref_section, $options, $hash_subst ) or return;
+        do_on_noaction( $ref_section, $options, $hash_subst ) or return;
     }
     return 1;
 }
diff -r 9034a7ecbb2d -r fc33cd0e9c62 lib/PFTools/Update/MKDIR.pm
--- a/lib/PFTools/Update/MKDIR.pm	Thu Aug 07 15:15:52 2014 +0200
+++ b/lib/PFTools/Update/MKDIR.pm	Thu Aug 07 18:31:27 2014 +0200
@@ -32,38 +32,40 @@
 use PFTools::Update::Common;
 
 our @EXPORT = qw(
-    Action_depends
-    Action_exec
+    action_depends
+    action_exec
 );
 
 our @EXPORT_OK = qw();
 
-sub Action_depends {
+sub action_depends {
     my ( $ref_section, $dest, $options ) = @_;
 
-    while ( $dest ne "/" && $dest ne "." ) {
+    while ( $dest ne q{/} && $dest ne q{.} ) {
         my $new_dest = dirname($dest);
-        $ref_section->{'depends'} .= " " . $new_dest
-            if ( $new_dest ne "/" || $new_dest ne "." );
+        if ( $new_dest ne q{/} || $new_dest ne q{.} ) {
+            $ref_section->{'depends'} .= q{ } . $new_dest;
+        }
         $dest = $new_dest;
     }
+    return;
 }
 
-sub Action_exec {
+sub action_exec {
     my ( $ref_section, $dest, $options, $hash_subst, $global_config ) = @_;
 
     my $cmp = 0;
 
     $hash_subst->{'SECTIONNAME'} = $dest;
-    unless ( -d $dest ) {
+    if ( !-d $dest ) {
         $cmp = 1;
         if ( $options->{'verbose'} || $options->{'simul'} ) {
-            Log(colored(q{(action needed)}, q{bold red}));
+            Log( colored( q{(action needed)}, q{bold red} ) );
         }
-        Do_on_config( $ref_section, $options, $hash_subst ) or return;
-        Do_before_change( $ref_section, $options, $hash_subst ) or return;
+        do_on_config( $ref_section, $options, $hash_subst ) or return;
+        do_before_change( $ref_section, $options, $hash_subst ) or return;
         if ( !$options->{'simul'} ) {
-            Do_moveold( $dest, $options );
+            do_moveold( $dest, $options );
             eval { make_path($dest); };
             if ($EVAL_ERROR) {
                 carp qq{ERROR: make_path($dest): $EVAL_ERROR};
@@ -71,10 +73,10 @@
             }
         }
     }
-    Do_chownmod( $ref_section, $dest, $options );
+    do_chownmod( $ref_section, $dest, $options );
     if ($cmp) {
-        Do_after_change( $ref_section, $options, $hash_subst ) or return;
-        Do_on_noaction( $ref_section, $options, $hash_subst ) or return;
+        do_after_change( $ref_section, $options, $hash_subst ) or return;
+        do_on_noaction( $ref_section, $options, $hash_subst ) or return;
     }
     return 1;
 }
diff -r 9034a7ecbb2d -r fc33cd0e9c62 lib/PFTools/Update/PURGEPKG.pm
--- a/lib/PFTools/Update/PURGEPKG.pm	Thu Aug 07 15:15:52 2014 +0200
+++ b/lib/PFTools/Update/PURGEPKG.pm	Thu Aug 07 18:31:27 2014 +0200
@@ -32,33 +32,34 @@
 use PFTools::Update::Common;
 
 our @EXPORT = qw(
-    Action_depends
-    Action_exec
+    action_depends
+    action_exec
 );
 
 our @EXPORT_OK = qw();
 
-sub Action_depends {
+sub action_depends {
 
     # Useless funciton but need to define
     return 1;
 }
 
-sub Action_exec {
+sub action_exec {
     my ( $ref_section, $dest, $options, $hash_subst, $global_config ) = @_;
 
-    $options->{'pkg_type'} = 'deb' unless ( $options->{'pkg_type'} );
+    if ( !$options->{'pkg_type'} ) {
+        $options->{'pkg_type'} = 'deb';
+    }
     my $name_filter = $ref_section->{'name_filter'};
     if ($name_filter) {
         $hash_subst->{'SECTIONNAME'} = $dest;
-        my $newdest = deferredlogpipe(
-            Subst_vars( $name_filter, $hash_subst )
-        );
-        unless ( defined $newdest ) {
+        my $newdest
+            = deferredlogpipe( Subst_vars( $name_filter, $hash_subst ) );
+        if ( !defined $newdest ) {
             carp qq{ERROR: Unable to apply name_filter $name_filter};
             return;
         }
-        unless ($newdest) {
+        if ( !$newdest ) {
             carp qq{ERROR: Empty result for name_filter $name_filter};
             return;
         }
@@ -66,26 +67,25 @@
     }
 
     my $status = Get_pkg_status( $options->{'pkg_type'}, $dest );
-    unless ($status) {
+    if ( !$status ) {
         carp qq{ERROR: Unable to retrieve status for $dest};
         return;
     }
 
     if ( $status->{'installed'} ) {
         if ( $options->{'verbose'} || $options->{'simul'} ) {
-            Log(colored(q{(action needed)}, q{bold red}));
+            Log( colored( q{(action needed)}, q{bold red} ) );
         }
-        Do_on_config( $ref_section, $options, $hash_subst ) or return;
-        Do_before_change( $ref_section, $options, $hash_subst ) or return;
-        if (!$options->{'simul'}
-            && !Purge_pkg( $options->{'pkg_type'}, $dest )
-            )
+        do_on_config( $ref_section, $options, $hash_subst ) or return;
+        do_before_change( $ref_section, $options, $hash_subst ) or return;
+        if (   !$options->{'simul'}
+            && !Purge_pkg( $options->{'pkg_type'}, $dest ) )
         {
             carp qq{ERROR: During purge for $dest};
             return;
         }
-        Do_after_change( $ref_section, $options, $hash_subst ) or return;
-        Do_on_noaction( $ref_section, $options, $hash_subst ) or return;
+        do_after_change( $ref_section, $options, $hash_subst ) or return;
+        do_on_noaction( $ref_section, $options, $hash_subst ) or return;
     }
     return 1;
 }
diff -r 9034a7ecbb2d -r fc33cd0e9c62 lib/PFTools/Update/REMOVEDIR.pm
--- a/lib/PFTools/Update/REMOVEDIR.pm	Thu Aug 07 15:15:52 2014 +0200
+++ b/lib/PFTools/Update/REMOVEDIR.pm	Thu Aug 07 18:31:27 2014 +0200
@@ -31,36 +31,37 @@
 use PFTools::Update::Common;
 
 our @EXPORT = qw(
-    Action_depends
-    Action_exec
+    action_depends
+    action_exec
 );
 
 our @EXPORT_OK = qw();
 
-sub Action_depends {
+sub action_depends {
     return 1;
 }
 
-sub Action_exec {
+sub action_exec {
     my ( $ref_section, $dest, $options, $hash_subst, $global_config ) = @_;
 
     # Is $options->{'simul'} necessary ?
     if ( !-e $dest ) {
         return 1;
-    } elsif ( !-d $dest && !$options->{'simul'} ) {
+    }
+    elsif ( !-d $dest && !$options->{'simul'} ) {
         carp qq{ERROR: $dest MUST BE a directory};
         return;
     }
     if ( $options->{'verbose'} || $options->{'simul'} ) {
-        Log(colored(q{(action needed)}, q{bold red}));
+        Log( colored( q{(action needed)}, q{bold red} ) );
     }
-    Do_on_config( $ref_section, $options, $hash_subst ) or return;
-    Do_before_change( $ref_section, $options, $hash_subst ) or return;
+    do_on_config( $ref_section, $options, $hash_subst ) or return;
+    do_before_change( $ref_section, $options, $hash_subst ) or return;
     if ( !$options->{'simul'} ) {
-        Do_moveold( $dest, $options );
+        do_moveold( $dest, $options );
     }
-    Do_after_change( $ref_section, $options, $hash_subst ) or return;
-    Do_on_noaction( $ref_section, $options, $hash_subst ) or return;
+    do_after_change( $ref_section, $options, $hash_subst ) or return;
+    do_on_noaction( $ref_section, $options, $hash_subst ) or return;
     return 1;
 }
 
diff -r 9034a7ecbb2d -r fc33cd0e9c62 lib/PFTools/Update/REMOVEFILE.pm
--- a/lib/PFTools/Update/REMOVEFILE.pm	Thu Aug 07 15:15:52 2014 +0200
+++ b/lib/PFTools/Update/REMOVEFILE.pm	Thu Aug 07 18:31:27 2014 +0200
@@ -31,36 +31,37 @@
 use PFTools::Update::Common;
 
 our @EXPORT = qw(
-    Action_depends
-    Action_exec
+    action_depends
+    action_exec
 );
 
 our @EXPORT_OK = qw();
 
-sub Action_depends {
+sub action_depends {
     return 1;
 }
 
-sub Action_exec {
+sub action_exec {
     my ( $ref_section, $dest, $options, $hash_subst, $global_config ) = @_;
 
     # Is $options->{'simul'} necessary ?
     if ( !-e $dest ) {
         return 1;
-    } elsif ( !-f $dest && !$options->{'simul'} ) {
+    }
+    elsif ( !-f $dest && !$options->{'simul'} ) {
         carp qq{ERROR: $dest MUST BE a file};
         return;
     }
     if ( $options->{'verbose'} || $options->{'simul'} ) {
-        Log(colored(q{(action needed)}, q{bold red}));
+        Log( colored( q{(action needed)}, q{bold red} ) );
     }
-    Do_on_config( $ref_section, $options, $hash_subst ) or return;
-    Do_before_change( $ref_section, $options, $hash_subst ) or return;
+    do_on_config( $ref_section, $options, $hash_subst ) or return;
+    do_before_change( $ref_section, $options, $hash_subst ) or return;
     if ( !$options->{'simul'} ) {
-        Do_moveold( $dest, $options );
+        do_moveold( $dest, $options );
     }
-    Do_after_change( $ref_section, $options, $hash_subst ) or return;
-    Do_on_noaction( $ref_section, $options, $hash_subst ) or return;
+    do_after_change( $ref_section, $options, $hash_subst ) or return;
+    do_on_noaction( $ref_section, $options, $hash_subst ) or return;
     return 1;
 }
 



More information about the pf-tools-commits mailing list