views:

653

answers:

2

Like many people, I can do a lot of things with PHP. One problem I do face constantly is that other people can do it much cleaner, much more organized and much more structured. This also results in much faster execution times and much less bugs.

I just finished writing a basic PHP Socket Server (the real core), and am asking you if you can tell me what I should do different before I start expanding the core. I'm not asking about improvements such as encrypted data, authentication or multi-threading.

I'm more wondering about questions like "should I maybe do it in a more object oriented way (using PHP5)?", or "is the general structure of the way the script works good, or should some things be done different?". Basically, "is this how the core of a socket server should work?"

In fact, I think that if I just show you the code here many of you will immediately see room for improvements. Please be so kind to tell me. Thanks!

#!/usr/bin/php -q
<?
// config
$timelimit = 180; // amount of seconds the server should run for, 0 = run indefintely
$address = $_SERVER['SERVER_ADDR']; // the server's external IP
$port = 9000; // the port to listen on
$backlog = SOMAXCONN; // the maximum of backlog  incoming connections that will be queued for processing

// configure custom PHP settings
error_reporting(1); // report all errors
ini_set('display_errors', 1); // display all errors
set_time_limit($timelimit); // timeout after x seconds
ob_implicit_flush(); // results in a flush operation after every output call

//create master IPv4 based TCP socket
if (!($master = socket_create(AF_INET, SOCK_STREAM, SOL_TCP))) die("Could not create master socket, error: ".socket_strerror(socket_last_error()));

// set socket options (local addresses can be reused)
if (!socket_set_option($master, SOL_SOCKET, SO_REUSEADDR, 1)) die("Could not set socket options, error: ".socket_strerror(socket_last_error()));

// bind to socket server
if (!socket_bind($master, $address, $port)) die("Could not bind to socket server, error: ".socket_strerror(socket_last_error()));

// start listening
if (!socket_listen($master, $backlog)) die("Could not start listening to socket, error: ".socket_strerror(socket_last_error()));

//display startup information
echo "[".date('Y-m-d H:i:s')."] SERVER CREATED (MAXCONN: ".SOMAXCONN.").\n"; //max connections is a kernel variable and can be adjusted with sysctl
echo "[".date('Y-m-d H:i:s')."] Listening on ".$address.":".$port.".\n";
$time = time(); //set startup timestamp

// init read sockets array 
$read_sockets = array($master);

// continuously handle incoming socket messages, or close if time limit has been reached
while ((!$timelimit) or (time() - $time < $timelimit)) {
    $changed_sockets = $read_sockets;
    socket_select($changed_sockets, $write = null, $except = null, null);
    foreach($changed_sockets as $socket) {
     if ($socket == $master) {
      if (($client = socket_accept($master)) < 0) {
       echo "[".date('Y-m-d H:i:s')."] Socket_accept() failed, error: ".socket_strerror(socket_last_error())."\n";
       continue;
      } else {
       array_push($read_sockets, $client);
       echo "[".date('Y-m-d H:i:s')."] Client #".count($read_sockets)." connected (connections: ".count($read_sockets)."/".SOMAXCONN.")\n";
      }
     } else {
      $data = @socket_read($socket, 1024, PHP_NORMAL_READ); //read a maximum of 1024 bytes until a new line has been sent
      if ($data === false) { //the client disconnected
       $index = array_search($socket, $read_sockets);
       unset($read_sockets[$index]);
       socket_close($socket);
       echo "[".date('Y-m-d H:i:s')."] Client #".($index-1)." disconnected (connections: ".count($read_sockets)."/".SOMAXCONN.")\n";
      } else {
       if ($data = trim($data)) { //remove whitespace and continue only if the message is not empty
        switch ($data) {
         case "exit": //close connection when exit command is given
          $index = array_search($socket, $read_sockets);
          unset($read_sockets[$index]);
          socket_close($socket);
          echo "[".date('Y-m-d H:i:s')."] Client #".($index-1)." disconnected (connections: ".count($read_sockets)."/".SOMAXCONN.")\n";
          break;
         default: //for experimental purposes, write the given data back
          socket_write($socket, "\n you wrote: ".$data);
        }
       }
      }
     }
    }
}
socket_close($master); //close the socket
echo "[".date('Y-m-d H:i:s')."] SERVER CLOSED.\n";
?>
A: 

Hey,

One small thing, personally i'd create a function for outputting instead of just using echo, that way its easy to turn it off, change the format etc.. eg

function log($message = '')
{
    echo '['.date('Y-m-d H:i:s').']'.$message;
}

and then you can use :

log("SERVER CREATED (MAXCONN: ".SOMAXCONN.").\n");

instead of

echo "[".date('Y-m-d H:i:s')."] SERVER CREATED (MAXCONN: ".SOMAXCONN.").\n";

Oh and be sure to use === instead of == otherwise you might get some odd results.

Paul Dixon
I should have mentioned it but the echos were just there for experimental purposes. Thanks though.I should have said that I'm questioning the effectiveness of the socket server's structure more. Eg. if I should really loop things like I'm doing or if there are better ways to do it.
Tom
A: 

I'd move your switch $data into a function as that will likely expand.

Also since it looks like you're using a text based protocol, might want to explode/strtok to get the first level command and check it against an array of valid commands. Could also have an array that describes what internal function to call and use call_user_func_array to dispatch the call.

NeuroScr