On Tue, Jul 10, 2001 at 03:08:51PM -0700, Piet wrote:
> Okay so I'm really new to perl but I've hacked together this program to
> parse /etc/passwd and email the login's manager every month.

Whenever I see "parsing /etc/passwd" my gut reaction is to tell the
programmer to use setpwent and getpwent, or better yet, User::pwent.  Try
it, it'll make your life easier.

Instead of:

    open(PASSWD, "/etc/passwd") || die(...);

    while (<PASSWD>) {
        my($name, $pass, ...) = split(":", $_);
        print "$name\n";
        # do other things with fields
    }

    close(PASSWD);


You would do:

    setpwent();

    while (my($name, $pass, ...) = getpwent()) { 
        print "$name\n";
        # do other things with fields
    }


Or:

    use User::pwent;

    setpwent();

    while (my $pw = getpwent()) {
        print $pw->name, "\n";
        # do other things with $pw
    }

See perldoc -f getpwent and perldoc User::pwent.



> Well it does just that but now I need to add some widget that will email
> the manager once with a list of all his personnel rather than one email
> per user in the passwd file.  I am at a standstill.  help!

Odd use of "widget" there..

Place the offending users in a hash, keyed by manager, then iterate over
that hash when you're done going through the passwd file.

Some commentary on your code below, along with suggestions on what to change
to get your multiple-users-in-one-email.  Some of the suggestions may be a
bit advanced yet for you, so feel free to ask for clarification.



> #!/usr/bin/perl

You forgot -w and use strict.  Always debug code with these on:

  #!/usr/bin/perl -w

  use strict;

Because of use strict you will now need to declare your variables with my.

> #
> #Script parses /etc/passwd and emails all users managers requesting
> status of subject user
> #
> #
> #
> ############################################################################
> 
> open(OUTPUT, "hostname|") || die "Can't stat hostname";
> $hostname=<OUTPUT>;
> close(OUTPUT);

You can replace this with:

  use Sys::Hostname qw(hostname);

  my $hostname = hostname();

With the added benefits that it's shorter and a little more portable.



> ############################################################################
> 
> # setup array of excluded system users (ie. peeps who don't have
> managers)
> ############################################################################
> 
> @exuser= ("root", "daemon", "bin", "sys", "adm", "lp", "uucp", "nuucp",
> "listen", "nobody", "noaccess", "nobody4");
> ############################################################################

You're using these values for doing lookups, but you're doing the lookup
linearly with grep.  This is slow, you should use a hash:

  my %exuser;
  @exuser{
    qw(root daemon bin sys adm lp uucp nuucp list nobody noaccess nobody4)
  } = ();


Also add:

  my %managers;

This will be a hash of lists, the keys are the managers, the values are an
array of users for that manager.


> 
> # read passwd file into array so weez can chunkonit
> ############################################################################
> 
> open (F, "/etc/passwd") || die "Could not open passwd file: $:";
> @password=<F>;
> ################################################################################
> 
> # perform the magic filtering necessary to pick out the names from the
> comment
> # field and email the managers requesting status of the user.
> ################################################################################
> 
>         foreach $line (@password)
>         {
>         ( $login, $pass, $uid, $gid, $Comm, $dir, $shell ) = split (/:/,
> $line);

As I suggested before, this should probably be replaced by getpwent calls.
Also, you're not using any of the values except $login and $Comm, so use
something like:

          my($login, $Comm) = (split(/:/, $line))[0,4];

Or:
          my($login, $Comm) = (getpwent)[0,4];

Meaning: retrieve the first and fifth fields from the split (or getpwent)
and assign them, respectively to $login and $Comm.


>         @User = split (/,/, $Comm);
> 
>         if (1 == scalar ( @User ))
>         {
> 
>                 next;
> 
>         }
>         if ( $Comm eq "" )
>         {
>                 next;
>         }

I'd replace this with:

          next if @User == 1 || $Comm eq '';

It's effectively the same code, but shorter and more to the point.


>         print "login=$login User=@User\n";
> 
>         if ( 1 != grep ( /$login/, @exuser ) )

Here's where we use the hash we initialized above:

          if (exists $exuser{$login})

>         {
> chomp ( $hostname );
> open( MAIL, "| /bin/mailx -s \"User Access\" -U $User[1]\@hotmail.com"
> );
>                 print MAIL <<"_END_OF_FILE_";
> Does $User[0] still require access to server $hostname?
> Please respond to my Team.
> _END_OF_FILE_

This entire chunk, the chomp, open, and print, should be replaced with:

            push(@{ $managers{$User[1]} }, $User[0]);

Also, you're chomping the hostname for each user, when you only need to
chomp it once (that is, if you don't use Sys::Hostname).


>         }
> 
> 
> }

Now, process the users:

  while (my($manager, $users) = each(%managers)) {
      open(MAIL, "| /bin/mailx -s \"User Access\" -U $manager\@hotmail.com");

      foreach my $user (@$users) {
          print MAIL "Does $user still require access to server $hostname?\n";
      }

      print MAIL "\n\nPlease respond to my Team.\n";
      close(MAIL);
  }


You may want to consider using a module, such as Mail::Mailer, for sending
the email.


Michael
--
Administrator                      www.shoebox.net
Programmer, System Administrator   www.gallanttech.com
--

Reply via email to