The first thing that strikes me here is.. why are you using printf everywhere? Also, rather than print,exit, you can more idiomatically use die.
Also, most of your globals needn't be.
Also, instead of lining up two dozen prints, it's better to use a heredoc. Actually, in your case, since you're printing the usage message that way, it would be even better to use Pod::Usage (part of the core since 5.8, IIRC). It plays well with Getopt::Long, too, which allows you to easily handle multiple get and delete options at once. Note that for all of the long option names, you can automatically use a one-letter abbreviation if it is unambiguous, so you lose no brevity over Getopt::Std.
As an side, passing passwords on the command line isn't wise. Any other using of the box can ps awx and steal it.
The following piece of code is really bizarre. For readability, the condition should just check @ARGV.
if ($#ARGV == -1) {
goto mailcon;
$nopts = 1;
} else {
$nopts = 0;
}
This is better written like so:
if (@ARGV) {
$nopts = 0;
} else {
goto mailcon;
$nopts = 1;
}
But the use of goto is strange, and the assignment following it will never be executed. The other goto also obscures the flow. There's no reason to use them there.
Further down, you check if the number of messages " eq '0E0'", but the whole point of the excercise of returning that strange seeming value is that you get a 0 that is true in boolean context so you can or die during object construction, but then still test if the number is == 0. So I'd replace it with that, because that's what the intent of the code is.
Many of your checks consist of something like if(error){print message}else{do stuff}. In Perl, it's much easier to say die "foo" if error; do stuff;. That usually means fewer levels of nesting to keep track of and usually makes the code more straightforward, so you should usually use it.
Finally, creating temporary files is a tricky business best left to File::Temp, which has been part of the core forever. Also, please see Two-arg open() considered dangerous. Note, as well, that you can easily use lexical variables in more recent Perls to hold file handles. This is preferrable to polluting your namespace with global filehandle symbols.
However, in this case, I'd not even use temporary files. You can spawn a pipe to less and feed the output to it directly, but why even bother? Just print the messages to STDOUT and let the user redirect them where he wants to (works even better if all regular messages (of which there is only one, "no messages" or "x messages on the server") are printed to STDERR). That's what he's got his Unix toolset for, anyway. In fact, this way you're generating bog standard mbox format output which can be fed to the stock mail handling tools.
So here's all that, in code.
#!/usr/bin/perl -w
=head1 NAME
popcheck - quick, cheap and dirty mail checker
=head1 SYNOPSIS
popcheck [-g msgnum] [-d msgnum] [-h]
Options:
--server Set POP3 server
--user Set POP3 username
--pass Set POP3 password
--get Get message
--del Delete message
--help Show manpage
The usage of (alternate) usernames and passwords are conditional on bo
+th the
username and password being specified (on the command line).
=head1 DESCRIPTION
Quick and cheap perl script to check mail on a remote
POP3 server. Born when mutt died ...
=head1 OPTIONS
=over 4
=item --server
The hostname of the POP3 server to contact.
If no default has been set in the script, this option is required.
=item --user
The username to log into the POP3 with.
=item --pass
The password to log into the POP3 with.
=item --get
The number of a message which is to be retrieved.
This option may be specified multiple times.
=item --del
The number of a message which is to be deleted.
This option may be specified multiple times.
=back
=head1 VERSION
$Revision: 1.1 $
=head1 COPYRIGHT
Originally written by Elfyn McBratney L<mailto:elfyn@emcb.co.uk>
and released under a "do what you like with it but don't blame me"
type license.
=cut
use strict;
use File::Basename;
use Getopt::Std;
use Net::POP3;
my $pop_host;
GetOptions(
'server=s' => \($pop_host = ''), # defaults could be supplied h
+ere
'user=s' => \(my $pop_user = ''),
'pass=s' => \(my $pop_pass = ''),
'get=i' => \(my @opt_get),
'del=i' => \(my @opt_del),
'help' => \(my $opt_help),
) and $pop_host and @ARGV == 0 or pod2usage(-verbose => 1);
pod2usage(-verbose => 2) if $opt_help;
my $pop = Net::POP3->new($pop_host, Timeout => 30)
or die "Failed to connect to server `$pop_host': $!\n";
my $msgnum = $pop->login($pop_user, $pop_pass)
or die "Failed to login on server `$pop_host' as user `$pop_user':
+ $!\n";
print STDERR $msgnum == 0
? "No messages on server\n"
: "$msgnum message(s) on server\n";
my $retrieved_messages = '';
for my $opt_get (@opt_get) {
die "You asked for message $opt_get but there are only $msgnum mes
+sages.\n"
if $opt_get > $msgnum;
my $msg = $pop->get($opt_get)
or die "Failed to get message $opt_get: $!\n";
$retrieved_messages .= join '', @$msg;
}
for my $opt_del (@opt_del) {
die "You asked for message $opt_del but there are only $msgnum mes
+sages.\n"
if $opt_del > $msgnum;
$pop->delete($opt_del);
}
print $retrieved_messages;
$pop->quit();
Now all it needs is a way to list the subjects of all mails in your inbox..
Makeshifts last the longest. | 