Andrei Faust Tanasescu wrote: > > Hello there list! Hello,
> I am eager to hear your feedback on this script I wrote. > Its reason of being is to be ran from cron (under Linux) so it warns root > and cuts internet conenction whenever download or upload reaches a certain > limit. This because our ISP bills us $10c for every megabyte of data over > their 6Gb monthly limit... > > So here it is: http://pages.infinit.net/davoid/dlimit > #!/usr/bin/perl -w > > use POSIX; Since ceil() is the only function you are using from POSIX you should import it only. use POSIX 'ceil'; > # Bandwidth download limiter. > # Stops your internet connection when you are close to start paying > # for extra megs. > # Andrei Faust Tanasescu (c) 2002 > # > > > > #################### CHANGE THE VALUES HERE TO MATCH YOUR SYSTEM ############## > > $netIface = "eth0"; # your network interface connected to the net. > $downloadLimit = "6144"; # your download limit in megabytes > $uploadLimit = "5120"; # upload limit in megabytes Numbers don't have to be quoted. my $downloadLimit = 6144; # your download limit in megabytes my $uploadLimit = 5120; # upload limit in megabytes > $lastStats = "/etc/lastStats"; > @actions = qw /kill mail/; # actions taken when limit is reached > > ############################################################################### > > my $download; > my $upload; > > sub main { main?? Perl is not C :-) > if (-e $lastStats) { > print "Loading stats\n"; > loadStats(); > } > else { > fetchProc(); > } > > > print "Upload at the moment is " . megs($upload) . " megs and download is " . >megs($download) . " megs.\n"; > print "You have " . downloadLeft(megs($download)) . " megs of download and " . >uploadLeft(megs($upload)) . " megs of upload left\n"; > > if( (downloadLeft(megs($download)) < 100) or > (uploadLeft(megs($upload)) < 100) ) { > takeAction(); > } > > saveStats(); > > if (haveToZero()) { > zeroTransfer(); > } > } > > sub fetchProc { > open INPUT, "/proc/net/dev" or die "Could __NOT__ fetch information about >your network interfaces."; You should include the $! variable in the error message so that you find out _why_ the error occurred. > while ($line = <INPUT>) { > if ($line =~ $netIface) { > last; > } > } > close INPUT; > > @tokens = split /:/, $line; > @info = split / /, $tokens[1]; #network information is in info > > $download = $info[0]; > $upload = $info[31]; It's a wonder that this works at all if it ever did! The contents of my /proc/net/dev looks like this: Inter-| Receive | Transmit face |bytes packets errs drop fifo frame compressed multicast|bytes packets errs drop fifo colls carrier compressed lo: 200 4 0 0 0 0 0 0 200 4 0 0 0 0 0 0 eth0: 3735232 4833 0 0 0 0 0 18 455300 5537 0 0 0 0 0 0 sit0: 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 A better way to parse the contents would be: my ($line) = grep s/^\s*eth0://, <INPUT>; ( $download, $upload ) = ( $line =~ /\d+/g )[0,8]; > > } > > sub loadStats{ > open DATA, $lastStats or die "$lastStats exists but can't open."; You should include the $! variable in the error message so that you find out _why_ the error occurred. > @lines = <DATA>; > close DATA; > > chomp $lines[0]; > chomp $lines[1]; > $download = $lines[0]; > $upload = $lines[1]; Since you only want the first two lines there is no point in reading the whole file into an array. chomp( $download = <DATA> ); chomp( $upload = <DATA> ); > # on a change de mois sans reboot !!! record !!! > if ($download == $upload and $download == 0) { Could also be written as: unless ( $download or $upload ) { > print "On a change de mois et 100% uptime!!! Felicitations!!"; > fetchProc(); > } > > } > > sub zeroTransfer { > open DATA, "> $lastStats" or die "Couln't write to $lastStats..."; You should include the $! variable in the error message so that you find out _why_ the error occurred. > print DATA 0 . "\n"; > print DATA 0 . "\n"; > } Why the two subs zeroTransfer() and saveStats() which do the same thing? > # > # called once a month, saves download/upload for that month > # > sub saveStats { > open DATA, "> $lastStats" or die > "Can't write to $lastStats\n Are you woot?"; > print DATA $download . "\n"; > print DATA $upload . "\n"; > close DATA; > } > > sub haveToZero { > my @dateInfo = localtime(time()); > my $dayNumber = $dateInfo[3]; There is no need for the array. my $dayNumber = (localtime)[3]; > if ($dayNumber == 1) { return 1; } > > else { return 0; } Or simply: return $dayNumber == 1; Or write the whole sub like this: sub haveToZero { (localtime)[3] == 1 } > } > # > # Transforms a number of bytes to megs > # > sub megs { > $_ = shift (@_); You should _NEVER_ modify a global variable like $_ in a subroutine. Use a lexically scoped variable. > return ceil((($_ / 1024) / 1024)); > } > > # > # Returns the number of megs left to download limit > # > sub downloadLeft { > $transfered = shift(@_); > return ceil($downloadLimit - $transfered); > } > > sub uploadLeft { > $transfered = shift(@_); > return ceil($uploadLimit - $transfered); > } > > # > # The function that actually does something ... > # > sub takeAction { > $message = "You are reaching your download/upload limit!!"; > > print $message; > foreach $action (@actions) { > if ($action =~ "kill") { You want to test for equality instead of using a regular expression. if ($action eq 'kill') { > `/sbin/ifconfig $netIface down`; If you have backticks in a void context then you probably want to use system instead. system( '/sbin/ifconfig', $netIface, 'down' ) == 0 or die "system /sbin/ifconfig failed: $?"; > } > elsif ($action =~ "mail") { elsif ($action eq 'mail') { > `echo "$message" | mail -s "$message" root\@localhost`; system( "echo '$message' | mail -s '$message' root\@localhost" ) == 0 or die "system mail failed: $?"; Or perhaps use a piped open: open MAIL, "| mail -s '$message' root\@localhost" or die "Cannot open pipe to mail: $!"; print MAIL $message; close MAIL or die "Cannot close pipe to mail: $!"; > } > } > } > > main() John -- use Perl; program fulfillment -- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]