in reply to Owner Permissions restore script

my $atime = $info->atime; # time of last access (seconds since epoch) my $mtime = $info->mtime; # time of last data modification my $ctime = $info->ctime; # time of last file status change

These variables are used exactly once. Why use them at all? All over the script you have lots and lots of variables that are completely superfluous. You don't need another new variable to store every intermediate result from each computation.

# if no argument given, default to current directory my $startdir = cwd(); @ARGV = $startdir unless @ARGV; # if argument given , overwrite cwd $startdir = "@ARGV"; # convert /usr/bin to _usr_bin for filename ($startdir = $startdir) =~ s/\//_/g;

That is quite unnecessarily convoluted. I'm not sure what that ($startdir = $startdir) is for, either. Just say

my $startdir = $ARGV[0] || cwd(); $startdir =~ s/\//_/g;
# get the current date to append to output filename my ($sec,$min,$hour,$mday,$mon,$year,$wday,$yday,$isdst)=localtime(tim +e); my $cyear = $year+1900; my $cmonth = $mon+1; if ( $cmonth < 10 ) {$cmonth = "0" . $cmonth;} my $cday = $mday; if ( $cday < 10 ) { $cday = "0" . $cday; } my $cdate = "_$cday" . "_$cmonth" . "_$cyear"; $txtfileout = "permrestore" . $startdir . $cdate;

Meet sprintf:

my ($sec,$min,$hour,$mday,$mon,$year,$wday,$yday,$isdst)=localtime(tim +e); my $txtfileout = sprintf "permrestore%s_%02d_%02d_%04d", $startdir, $m +day, $mon, $year;

But using the POSIX::strftime() function would remove all the fiddling completely:

use POSIX qw( strftime ); my $txtfileout = strftime "permrestore" . $startdir . "_%d_%m_%Y", loc +altime;

Btw — it's good habit to YYYY-MM-DD dates when you're doing things on computers. There are strange places in the world where dates are written MM-DD-YYYY instead of DD-MM-YYYY, so the year-last notation is ambiguous (does 04_05_2004 mean April, 5 or May, 4?). Also, YYYY-MM-DD naturally sorts correctly when you list the contents of a directory, whereas the opposite ordering makes it a mess.

# subroutines sub time_as_date($) { my ($time) = @_; my $date = localtime($time); return $date; }

What's the point? You're passing your parameter through localtime and returning the result. Why not call localtime directly?

my $size = $info->size; # size in bytes # ... my $adate = time_as_date($atime); my $mdate = time_as_date($mtime); my $cdate = time_as_date($ctime);

You don't even use any of these other than $mdate. Why build the rest?

my $perms = sprintf("%04o", $mode & 07777); # ... print "\# ORIG PERM = $filename\t$uid\t$gid\t$perms\t$mdate";

These are the first mentions of $mode, $uid and $uid in your script… where'd they come from? And why have an extra variable for them at all? Also you say $filename here for what was $name before.

sub time_as_date($) # ... sub print_info($)

You shouldn't be using prototypes if you don't know what they do. And will almost never use them once you do. Read Far More Than Everything You've Ever Wanted to Know about Prototypes in Perl).

use File::Find (); # ... sub find(&@) { &File::Find::find }

That seems silly. Why not just import the function? Particularly when you go ahead and import something yourself:

*name = *File::Find::name;

You probably wanted just the scalar, btw:

*name = \$File::Find::name;

Finally, this may just be personal preference, but I find games like this a bit silly:

print "\n Do you want to continue (y/n)? "; $answer = <STDIN>;

If, as it's likely, you've only done that because you might forget how to call the program, so it shouldn't continue on if you only wanted to see the usage info, then I'd use Getopt::Std or Getopt::Long to pick up on a -h parameter, which is the common convention.

Also, why write the output to both a specifically opened file as well as the standard output? Just write it to standard output and the user can redirect it as they see fit. If they want to see the output and at the same time write it to a file, that's what tee(1) is for: restoreperm | tee restorescript or even restoreperm | tee restore-`date +%Y%m%d`. I find this much more flexible.

Given all that advice, all that's left of your code in the end is:

#!/usr/bin/perl use strict; use warnings; use File::Find; find sub { my $filename = $File::Find::name; my ( $mode, $uid, $gid, $mtime ) = ( stat $filename )[ 2, 4, 5, 9 +]; my $mdate = localtime( $mtime ); # discard file type info from 'mode' and put in usual numeric form +at my $perms = sprintf( "%04o", $mode & 07777 ); print "# ORIG PERM = $filename\t$uid\t$gid\t$perms\t$mdate\n"; print "chmod $perms $filename\n"; print "chown $uid:$gid $filename\n"; }, @ARGV ? @ARGV : '.';

A little bit shorter, no?

Also, it does exactly one job. If you still really want the automatic filename computation and such that you had before, just wrap it with a little shell:

#!/bin/bash if [ "$1" ] ; then DIR="$1" ; else DIR="$PWD" ; fi SCRIPT="permrestore${DIR//\//_}$( date +_%d_%m_%Y )"; permrestore "$DIR" | tee "$SCRIPT" chmod a+x "$SCRIPT"

I know I'm on the right track when by deleting code I'm adding functionality. —John Carter

Considering that quote, I'd be likely to replace the entire Perl script with a single call to GNU find(1):

#!/bin/bash FORMAT="# ORIG PERM = %p\t%U\t%G\t%m\t%Tc" FORMAT="$FORMAT\nchmod %m %p" FORMAT="$FORMAT\nchown %U:%G %p" FORMAT="$FORMAT\n" if [ "$1" ] ; then DIR="$1" ; else DIR="$PWD" ; fi SCRIPT="permrestore${DIR//\//_}$( date +_%d_%m_%Y )"; find "$DIR" -printf "$FORMAT" | tee "$SCRIPT" chmod a+x "$SCRIPT"

Sometimes, shell is better than Perl.

Makeshifts last the longest.