FAQ
"Nz" == Niko zuna writes:
Nz> #!/usr/bin/perl

Nz> # Needed pkg

no need for comments like that.
Nz> use Getopt::Std;

use Getopt::Long instead. you can still have short names but also long
names.

Nz> use strict "vars";

why 'vars'? just plain strict is better.

Nz> # Global variables

again, a useless comment. this is like saying $i++ # add 1 to $i

Nz> my $VERBOSE = 0;
Nz> my $DEBUG = 0;

Nz> ################################################################
Nz> # handle flags and
Nz> # Example: c == "-c", c: == "-c argument"

Nz> my $opt_string = 'hvdk:p:';
Nz> getopts( "$opt_string", \my %opt ) or usage() and exit 1; # exit other than
Nz> 0 = error!!

be careful how you paste code into email or other places. that word
wrapped and would break if i tried to cut/paste it to run here.

Nz> # print help message if -h is invoked
Nz> if ( $opt{'h'} ){
Nz> usage();
Nz> exit 0;
Nz> }

Nz> $VERBOSE = 1 if $opt{'v'};
Nz> $DEBUG = 1 if $opt{'d'};
Nz> my $PORT = $opt{'p'};

Nz> if ( not $PORT ) {
Nz> print "You need to apply a port number";
Nz> usage();
Nz> exit 1;
Nz> }

put all that arg processing into a sub. it makes it easier to manage and
also to read the code. i call it process_options() and put it at the
bottom of the script out of the way.

Nz> # main program content
Nz> use IO::Socket;
Nz> $SIG{"CHILD"} = "IGNORE"; # Enable us to not make the children hangout and
Nz> wait for the parent to die, (children will die)

Nz> my $sock = new IO::Socket::INET(

use direct method calls. this was discussed here very recently.

IO::Socket::INET->new(


Nz> LocalPort => $PORT,
Nz> Proto => 'tcp',
Nz> Listen => SOMAXCONN,
Nz> Reuse => 1
Nz> );

Nz> if ( $sock ) {
Nz> print "Waiting for clients...\n";
Nz> while ( my $connection = $sock->accept() ){
Nz> my $pid = fork(); # how the server knows if the porc is parent (pid =
Nz> xxx) or child (0)
Nz> if ( $pid == 0 ) {
Nz> my $client = $connection->peerhost(); # use $connection instead of
Nz> $sock to get ipaddr
Nz> print "Connection to the $client\n";

Nz> my $buffer = <$connection>;
Nz> chomp $buffer;
Nz> print "Client said: '$buffer'\n";
Nz> # sleep 7;

Nz> print $connection "Message recived: $buffer\n";

you only read one line there. if you want to read multiple lines you
need to loop until you detect eof or some end of command line you send
from the client.

Nz> close($connection);
Nz> exit; # server child should die after finish the conversation
Nz> } else {
Nz> close($connection);
Nz> next;
Nz> }
Nz> }
Nz> } else {
Nz> die "Error: $!\n";
Nz> }

that should be:

$socket or die "Error: $!\n";

then the good socket code follows inline with no block. no need for
if/else and blocks and extra indents.


Nz> close($sock);

Nz> verbose("verbose\n");
Nz> debug("noe\n");



Nz> ##########################################
Nz> # Helper routines

again, useless comments.

put the process_options sub down here. it will be easier to edit these
if they are near each other.

Nz> sub usage {
Nz> # print the correct use of this script
Nz> print "Usage:\n";
Nz> print "-h Usage\n";
Nz> print "-v Verbose\n";
Nz> print "-d Debug\n";
Nz> }

Nz> sub verbose {
Nz> print $_[0] if ( $VERBOSE or $DEBUG );
Nz> }

Nz> sub debug {
Nz> print $_[0] if ( $DEBUG );
Nz> }


many of the comments above are also for the client code.


Nz> my $COMMAND = $opt{'c'};

Nz> if ( not ( $PORT and $SERVER )){
Nz> print "You need to supply a port number and hostname\n";
Nz> usage();
Nz> exit 1;
Nz> }


Nz> # main program content

Nz> use IO::Socket;

Nz> my $connection = new IO::Socket::INET(
Nz> PeerAddr => $SERVER,
Nz> PeerPort => $PORT,
Nz> Proto => 'tcp'
Nz> ) or die "Failed to connect: $!\n";


Nz> if ( $connection ) {
Nz> print "Connection etablished...\n";

Nz> # print $connection "hello world!\n";
Nz> # print $connection "Isabelle says hello\n";

Nz> my $load = `uptime | awk '{print \$8 " " \$9 " " \$10 " " \$11 " "
Nz> \$12}'`;
Nz> my $uptime = `uptime | awk '{print \$2 " " \$3 " " \$4}'`;
Nz> my $mem_tot = `cat /proc/meminfo | grep MemTotal`;

why execute all the command when only one is needed? this is a waste of
cpu.

Nz> if ($COMMAND =~ m/load/){
Nz> print $connection $load;
Nz> }

those are multiple lines of text. you print them to the socket but the
server only reads one line and closes the connection.

Nz> if ($COMMAND =~ m/uptime/){
Nz> print $connection $uptime;
Nz> }

Nz> if ($COMMAND =~ m/memory/){
Nz> print $connection "$mem_tot\n";
Nz> }

Nz> if ($COMMAND =~ m/user:/){

use a dispatch table (google for that). then you have a sub for each
command which will execute the command and send the output to the
socket. or even better, return the output as a string and the main code
(in only one place) sends the text to the socket.

Nz> while ($COMMAND){
Nz> my @array = split(":", $COMMAND);

don't name things @array, it tells me nothing about the usage.

Nz> my $user = $array[1];

that is a poor way to get the user name. a regex does it simpler:

my( $user ) = $COMMAND =~ /:(\w+)/ or die "no user in $COMMAND" ;

Nz> my $ps = `ps aux | grep $user | wc -l`;

why shell out for those other commands? perl can do them well. also
there is a module to get ps output for you.

Nz> print $connection $ps;
Nz> sleep 7;
Nz> }
Nz> }

Nz> # $mem_free\n$mem_buff\nswa_tot\n$swa_free\n";
Nz> # print $connection `$COMMAND`;

Nz> my $response = <$connection>;

Nz> print "Server replied: '$response'\n";

Nz> close($connection);
Nz> }


Nz> ##########################################
Nz> # Helper routines

Nz> sub usage {
Nz> # print the correct use of this script
Nz> print "Usage:\n";
Nz> print "-h Usage\n";
Nz> print "-v Verbose\n";
Nz> print "-d Debug\n";
Nz> print "-p Port\n";
Nz> print "-H Host/Server\n";

there is no usage for -c. this is why keeping the opts processing near
the usage would be a good thing. you would keep them in sync better.


uri

--
Uri Guttman ------ uri@stemsystems.com -------- http://www.sysarch.com --
----- Perl Code Review , Architecture, Development, Training, Support ------
--------- Gourmet Hot Cocoa Mix ---- http://bestfriendscocoa.com ---------

Search Discussions

Discussion Posts

Previous

Related Discussions

Discussion Navigation
viewthread | post
posts ‹ prev | 2 of 2 | next ›
Discussion Overview
groupbeginners @
categoriesperl
postedApr 10, '10 at 7:32a
activeApr 10, '10 at 8:01a
posts2
users2
websiteperl.org

2 users in discussion

Uri Guttman: 1 post Niko zuna: 1 post

People

Translate

site design / logo © 2021 Grokbase