Can you comment on my implementation?

trivial comment: please prefix all public identifiers
with sdlnet_ or something like that. net_ is too generic,
and some of your public routines lack prefixes entirely.

slightly more useful comments:

  1. why do you queue incoming udp messages? Just pass 'em straight back.
    And don’t let 'em tell you what channel to listen to - you tell them where
    the
    packet came from!

  2. might want to offer a vector interface - user gives
    you an array of sdlnet_pktbuf_t’s, you fill 'em.

  3. mixing tcp and udp IMHO is a bad idea. It makes it hard to support
    NAT’s.
    But I agree TCP has a lot of advantages for some stuff. e.g.
    if you implement your own reliable transport over UDP, you’re
    responsible for handling a lot of stuff just right.

  4. your integer portability routines are not the prettiest. Gotta have 'em,
    though.
    I might be tempted to move them out into a separate module.

  5. you should provide a way to get a vector of all the network file
    descriptors
    so users can call select.

More later, perhaps.

  • Dan> -----Original Message-----

From: Sam Lantinga [SMTP:slouken at devolution.com]
Sent: Monday, October 26, 1998 11:04 AM
To: @Kegel_Dan
Subject: Can you comment on my implementation?

I haven’t even compiled it yet, but can you see any glaring flaws in
my network library implementation?

It’s designed to provide TCP and virtual UDP connections in a platform
independent manner.
In particular is my method of handling the virtual connections ignorant?
Just like TCP, there’s an “accept” channel for UDP, which collects packets
which don’t match any other channels.
It seems like a lot of overhead…

/*
NETLIB: An example cross-platform network library for use with SDL
Copyright © 1997 Sam Lantinga

This program is free software; you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
the Free Software Foundation; either version 2 of the License, or
(at your option) any later version.

This program is distributed in the hope that it will be useful,
but WITHOUT ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
GNU General Public License for more details.

You should have received a copy of the GNU General Public License
along with this program; if not, write to the Free Software
Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307

USA

Sam Lantinga
5635-34 Springhouse Dr.
Pleasanton, CA 94588 (USA)
slouken at devolution.com

*/

/* Include normal system headers */
#include <stdio.h>
#include <stdlib.h>
#include <string.h>

/* Include system network headers /
#if defined(WIN32) || defined(WIN32)
#define Win32_Winsock
#include <windows.h>
#else /
UNIX /
#include <unistd.h>
#ifndef BEOS
#include <arpa/inet.h>
#endif
#include <netinet/in.h>
#include <netdb.h>
#include <sys/socket.h>
#endif /
WIN32 */

#include “SDLnet.h”

/* System-dependent definitions */
#ifdef Win32_Winsock
#define handle SOCKET
#else
#define closesocket close
#define handle int
#define INVALID_SOCKET -1
#define SOCKET_ERROR -1
#endif

/* The network channel /
#define MAX_HOST_LEN sizeof(Uint32) /
32-bit IPv4 address */
struct net_address {
Uint8 host[MAX_HOST_LEN];
int len;
Uint16 port;
};
struct net_packet {
char data; / The packet data (malloc()'d) /
int len; /
The length of the packet data /
struct net_address src; /
The source address of the packet
/
};
struct net_channel {
int active; /
non-zero if this channel is in
use /
handle id; /
System-dependent channel id /
transport type; /
Type of network transport /
struct net_address address; /
Remote address of this channel */
int queueidx;
int queuelen;
struct net_packet queue[NET_MAX_QUEUE];
} channels[NET_MAX_CHANNELS];

/* Initialize/Cleanup the network API /
int Net_Init(void)
{
#ifdef Win32_Winsock
/
Start up the windows networking */
WORD version_wanted = MAKEWORD(1,1);
WSADATA wsaData;

if ( WSAStartup(version_wanted, &wsaData) != 0 ) {
SDL_SetError(“Couldn’t initialize Winsock 1.1\n”);
return(-1);
}
#endif
memset(channels, 0, sizeof(channels));
return(0);
}
void Net_Quit(void)
{
int i;

/* Close any outstanding channels /
for ( i=0; i<MAX_CHANNELS; ++i ) {
if ( channels[i].status != CHANNEL_INACTIVE ) {
Net_Close(i);
}
}
#ifdef Win32_Winsock
/
Clean up windows networking */
if ( WSACleanup() == SOCKET_ERROR ) {
if ( WSAGetLastError() == WSAEINPROGRESS ) {
WSACancelBlockingCall();
WSACleanup();
}
}
#endif
}

/* Open a communication channel of ‘proto’ transport type.
If ‘remote’ is NULL, this creates a local server port.
If ‘remote’ is not NULL, a connection to the remote address and port
is attempted. Note that for unreliable connections, there may be
nothing listening at the other end when the channel is created.
The channel number (0-31) is returned if successful, or a negative
number on error.
*/
int Net_Open(transport proto, char *remote, Uint16 port)
{
int which;
struct net_channel *channel

/* UDP/TCP socket specific variables */
int socket_type;
struct sockaddr_in sock_addr;

/* Find a free channel */
for ( which=0; which<MAX_CHANNELS; ++which ) {
if ( ! channels[which].active ) {
break;
}
}
if ( which == MAX_CHANNELS ) {
Net_SetError(“No free channels (%d maximum)”, MAX_CHANNELS);
which = -1;
goto net_done;
}
channel = &channels[which];
memset(channel, 0, sizeof(*channel));

/* Get the parameters for the appropriate transport type */
switch (address->type) {
case PROTO_UDP:
socket_type = SOCK_DGRAM;
break;
case PROTO_TCP:
socket_type = SOCK_STREAM;
break;
default:
Net_SetError(“Unknown address transport”);
which = -1;
goto net_done;
}

/* Open the socket */
channel->id = socket(AF_INET, socket_type, 0);
if ( channel->id == INVALID_SOCKET ) {
Net_SetError(“Couldn’t create socket”);
which = -1;
goto net_done;
}

/* Connect to remote, or bind locally, as appropriate */
if ( remote ) {
memset(&sock_addr, 0, sizeof(sock_addr));
sock_addr.sin_family = AF_INET;
sock_addr.sin_addr.s_addr = inet_addr(remote);
if ( sock_addr.sin_addr.s_addr == INADDR_NONE ) {
struct hostent *hp;

  	hp = gethostbyname(remote);
  	if ( hp == NULL ) {
  		Net_SetError("Invalid host: %s", remote);
  		which = -2;
  		goto net_done;
  	}
  	memcpy(&sock_addr.sin_addr, hp->h_addr,

hp->h_length);
}
sock_addr.sin_port = SDL_SwapBE16(port);

  /* Connect to the remote host */
  if ( connect(channel->id, (struct sockaddr *)&sock_addr,
  		sizeof(sock_addr)) == SOCKET_ERROR ) {
  	Net_SetError("Couldn't connect to remote host");
  	which = -2;
  	goto net_done;
  }
  /* Fill in the channel host address */
  channel->type = proto;
  channel->address.len = sizeof(Uint32);
  memcpy(channel->address.host, &sock_addr.sin_addr.s_addr,
  				channel->address.len);
  channel->address.port = sock_addr.sin_port;

} else {
memset(&sock_addr, 0, sizeof(sock_addr));
sock_addr.sin_family = AF_INET;
sock_addr.sin_addr.s_addr = INADDR_ANY;
sock_addr.sin_port = SDL_SwapBE16(port);

  /* Bind the socket for listening */
  if ( connect(channel->id, (struct sockaddr *)&sock_addr,
  		sizeof(sock_addr)) == SOCKET_ERROR ) {
  	Net_SetError("Couldn't bind to local port");
  	which = -2;
  	goto net_done;
  }
  if ( proto == PROTO_TCP ) {
  	if ( listen(channel->id, 5) == SOCKET_ERROR ) {
  		Net_SetError("Couldn't listen to local

port");
which = -2;
goto net_done;
}
}
/* Fill in the channel host address */
channel->type = proto;
channel->address.len = 0;
channel->address.port = sock_addr.sin_port;
}

/* Cleanup and return the channel */
net_done:
if ( which >= 0 ) {
channel->active = 1;
} else
if ( which == -2 ) {
close(channel->id);
}
return(which);
}

/* Verify the channel id and return a pointer to the associated structure
*/
static struct net_channel *GetChannel(int which)
{
if ( (which < 0) || (which >= NET_MAX_CHANNELS) ||
! channels[which].active ) {
Net_SetError(“Invalid network channel”);
return(NULL);
}
return(&channels[which]);
}

/* Utility function for Net_Accept() on a UDP channel */
static void CopyPackets(struct net_channel *src, struct net_channel *dst)
{
struct net_packet packets[NET_MAX_QUEUE];
int i, maxi;
int srcidx, dstidx;

/* Brute force, copy the packets to a temporary array /
i = src->queuelen;
srcidx = src->queueidx;
dstidx = 0;
maxi = srcidx+i
if ( maxi < NET_MAX_QUEUE ) {
memcpy(&packets[dstidx], &src->queue[srcidx],
i
sizeof(packets[i]));
} else {
i = NET_MAX_QUEUE-srcidx;
memcpy(&packets[dstidx], &src->queue[srcidx],
isizeof(packets[i]));
srcidx = 0;
dstidx += i;
i = maxi-NET_MAX_QUEUE;
memcpy(&packets[dstidx], &src->queue[srcidx],
i
sizeof(packets[i]));
}

/* Copy the first packet address to the destination */
memcpy(&dst->address, &packets[0].address, sizeof(dst->address));

/* Now, copy them all back to the appropriate channel /
maxi = src->queuelen;
srcidx = 0;
src->queueidx = 0;
src->queuelen = 0;
dstidx = 0;
dst->queueidx = 0;
dst->queuelen = 0;
for ( i=0; i<maxi; ++i ) {
if ( memcmp(&packets[i].address, &dst->address,
sizeof(&dst->address)) == 0 ) {
++dst->queuelen;
memcpy(&dst->queue[dstidx++], &packets[i],
sizeof(packets[i]));
} else {
++src->queuelen;
memcpy(&src->queue[srcidx++], &packets[i],
sizeof(packets[i]));
}
}
/
Done! */
return;
}

/* Accept a connection on the given network channel.
This only works on channels that have been opened as server channels.
If ‘remote’ is not NULL, the hostname of the remote system, up to
’rlen’
bytes, will be stored in the area pointed to by ‘remote’.
This returns a new channel for the connection, or -1 on error.
*/
int Net_Accept(int which, char *remote, int rlen)
{
struct net_channel *server;
struct net_channel *channel;

/* Make sure the channel is valid */
if ( (server=GetChannel(which)) == NULL ) {
return;
}

/* Find a free channel */
for ( which=0; which<MAX_CHANNELS; ++which ) {
if ( ! channels[which].active ) {
break;
}
}
if ( which == MAX_CHANNELS ) {
Net_SetError(“No free channels (%d maximum)”, MAX_CHANNELS);
which = -1;
goto net_done;
}
channel = &channels[which];
memset(channel, 0, sizeof(*channel));

/* What kind of accept do we perform? /
switch (server->type) {
case PROTO_UDP: {
/
Wait for a new packet /
UDP_Recv(server->id, server, NULL, 0);
if ( server->queuelen == 0 ) {
return(-1);
}
/
Set address and queue packets for the new channel
/
channel->id = server->id;
memcpy(&channel->address,
&server->queue[server->queueidx].address,
sizeof(channel->address));
CopyPackets(server, channel);
channel->type = PROTO_UDP;
}
break;
case PROTO_TCP: {
/
Use the sockets call */
struct sockaddr_in sock_addr;
int alen;

  	alen = sizeof(sock_addr);
  	channel->id = accept(server->id,
  			(struct sockaddr *)&sock_addr,

&alen);
if ( channel->id == SOCKET_ERROR ) {
Net_SetError(“accept() failed”);
which = -1;
goto net_done;
}
memcpy(channel->address.host,
&sock_addr.sin_addr.s_addr, sizeof(Uint32));
channel->address.len = sizeof(Uint32);
channel->address.port = sock_addr.sin_port;
channel->type = PROTO_TCP;
}
break;
default: {
Net_SetError(“Unknown channel transport”);
which = -1;
goto net_done;
}
}
if ( remote ) {
ResolveHost(channel, remote, rlen);
}

/* Cleanup and return the channel */
net_done:
if ( which >= 0 ) {
channel->active = 1;
} else
if ( which == -2 ) {
close(channel->id);
}
return(which);
}

/* Send data over a datagram connection to the remote side */
int UDP_Send(struct net_channel *channel, char *data, int len)
{
struct sockaddr_in sock_addr;
int i;

sock_addr.sin_family = AF_INET;

memcpy(&sock_addr.sin_addr.s_addr,channel->address.host,sizeof(Uint32));
sock_addr.sin_port = channel->address.port;
i = sizeof(sock_addr);
return(sendto(channel->id, data, len, 0, &sock_addr, i));
}
/* Send data over a stream connection to the remote side */
int TCP_Send(struct net_channel *channel, char data, int maxlen)
{
return(recv(channel->id, data, maxlen, 0));
}
/
Send a data over the given channel
If the data is sent over a datagram channel, the data is sent as a
single datagram. If the data is sent over a stream channel, the data
is added to the network stream.
This returns the length of the data sent, or -1 on error.
*/
int Net_Send(int which, char *data, int maxlen)
{
int len;
struct net_channel *channel;

/* Make sure the channel is valid */
if ( (channel=GetChannel(which)) == NULL ) {
return(-1);
}
if ( channel->address.host == NULL ) {
Net_SetError(“You cannot send on a server channel”);
return(-1);
}

/* What kind of accept do we perform? */
switch (channel->address->type) {
case PROTO_UDP: {
len = UDP_Send(channel, data, maxlen);
}
break;
case PROTO_TCP: {
len = TCP_Send(channel, data, maxlen);
}
break;
default: {
Net_SetError(“Unknown channel transport”);
len = -1;
}
}
return(len);
}

/* Return true if an internet address matches one of ours */
static int MatchInetAddress(struct sockaddr_in *sock_addr,
struct net_address *address)
{
if ( (address->len == sizeof(Uint32)) &&

(memcmp(&sock_addr->sin_addr.s_addr,address->host,sizeof(Uint32))
== 0) && (sock_addr.sin_port == address->port) ) {
return(1);
} else {
return(0);
}
}
/* Get data from a datagram connection and queue it on the proper channel
*/
int UDP_Recv(handle id, struct net_channel *channel, char *data, int
maxlen,
struct net_address *address)
{
struct net_channel *queued;
int i, len;
char *packet;

/* If there’s a packet queued, return it immediately */
if ( (channel != NULL) && (channel->queuelen > 0) ) {
i = channel->queueidx;
len = channel->queue[i].len;
if ( data != NULL ) {
if ( len > maxlen ) {
len = maxlen;
}
memcpy(data, channel->queue[i].data, len);
free(channel->queue[i].data);
channel->queuelen -= 1;
channel->queueidx = (i+1)%NET_MAX_QUEUE;
}
return(len);
}

/* Suck packets from the network for the channel */
packet = NULL;
do {
struct sockaddr_in sock_addr;

  /* Allocate a network packet */
  if ( packet == NULL ) {
  	packet = (Uint8 *)malloc(NET_MAX_PACKET);
  	if ( packet == NULL ) {
  		Net_SetError("Out of memory");
  		return(-1);
  	}
  }

  /* Read from the network */
  memset(&sock_addr, 0, sizeof(sock_addr));
  i = sizeof(sock_addr);
  len = recvfrom(id, packet, NET_MAX_PACKET,
  		(struct sockaddr *)&sock_addr, &i);
  if ( len < 0 ) {
  	Net_SetError("Error reading from network");
  	return(-1);
  }

  /* Look to see which channel it should go on */
  queued = NULL;
  for ( i=0; i<NET_MAX_CHANNELS; ++i ) {
  	/* If we have an address match, queue the packet */
  	if (channels[i].active &&

MatchInetAddress(&sock_addr,&channels[i].address)) {
queued = &channels[i];
break;
}
}
/* See if there’s a server channel we should queue it on /
if ( queued == NULL ) {
for ( i=0; i<NET_MAX_CHANNELS; ++i ) {
if ( channels[i].active &&
(channels[i].id == id) &&
(channels[i].address.len == 0) ) {
queued = &channels[i];
break;
}
}
}
/
If queued is still NULL, we drop the packet /
if ( queued != NULL ) {
/
Bypass the queue for the desired channel */
if ( (queued == channel) && (data != NULL) ) {
if ( len > maxlen ) {
len = maxlen;
}
memcpy(data, packet, len);
} else
if ( queued->queuelen < NET_MAX_QUEUE ) {
i = queued->queueidx+queued->queuelen;
i %= NET_MAX_QUEUE;
queued->queue[i].len = len;
queued->queue[i].data = packet;
if ( queued->address.len == 0 ) {
queued->queue[i].address.len =
sizeof(Uint32);

memcpy(queued->queue[i].address.host,
&sock_addr.sin_addr.s_addr,

queued->queue[i].address.len);
queued->queue[i].address.port =
sock_addr.sin_port;
}
packet = NULL;
queued->queuelen += 1;
}
}
} while ( channel && (queued != channel) );

/* If we have any leftover packet data, free it /
if ( packet != NULL ) {
free(packet);
}
return(len);
}
/
Get data from a stream connection and return it */
int TCP_Recv(struct net_channel *channel, char data, int maxlen)
{
return(recv(channel->id, data, maxlen, 0));
}
/
Wait for data on the given channel.
‘maxlen’ should be set to the size of the buffer pointed to by ‘data’.
Any datagram packet that exceeds maxlen in length will be truncated.
Any stream packet that exceeds maxlen in length will return the unread
portion in the next call to Net_Recv().
This returns the length of the returned data, 0 if the connection was
closed, or -1 on error.
*/
int Net_Recv(int which, char *data, int maxlen)
{
int len;
struct net_channel *channel;

/* Make sure the channel is valid */
if ( (channel=GetChannel(which)) == NULL ) {
return(-1);
}
if ( channel->address.host == NULL ) {
Net_SetError(“You cannot recv on a server channel”);
return(-1);
}

/* What kind of accept do we perform? */
switch (channel->address->type) {
case PROTO_UDP: {
len = UDP_Recv(channel->id, channel, data, maxlen);
}
break;
case PROTO_TCP: {
len = TCP_Recv(channel, data, maxlen);
}
break;
default: {
Net_SetError(“Unknown channel transport”);
len = -1;
}
}
return(len);
}

/* Wait for packets available on the channel mask designated by 'which’
If timeout is not 0, Net_Wait() will return 0 if no packets have been
received on the designated channels after ‘timeout’ milliseconds.

This function returns the number of connections that have data
available,
or -1 if there was a problem.
*/
int Net_Wait(NetCMASK *which, Uint32 timeout)
{
fdset tcpset, udpset;
int i, maxfd;
struct timeval tv;

/* Set up the select() call, based on requested mask */
FD_ZERO(&mask); FD_ZERO(&udpset);
maxfd = 0;
for ( i=0; i<MAX_NET_CHANNELS; ++i ) {
if ( Net_ISSET(i, which) ) {
if ( channels[i].active ) {
if ( maxfd < channels[i].id ) {
maxfd = channels[i].id;
}
FD_SET(channels[i].id, &mask);
} else {
Net_CLR(i, which);
}
}
}
tv.tv_sec = timeout/1000;
tv.tv_usec = (timeout%1000)*1000;

/* Select! */
do {
select(maxfd, &mask, NULL, NULL, &tv);
} while ( errno == EINTR );

/* See which channels are ready for reading */
for ( i=0; i<MAX_NET_CHANNELS; ++i ) {
if ( Net_ISSET(i, which) ) {
switch (channels[i].type) {
case PROTO_UDP:
if ( FD_ISSET(channels[i].id, &mask)
) {
UDP_Recv(channels[i].id,
NULL, NULL,
0);
}

}

/* Close a network channel */
void Net_Close(int which)
{
struct net_channel *channel;

/* Make sure the channel is valid */
if ( (channel=GetChannel(which)) == NULL ) {
return;
}

switch (channel->type) {
case PROTO_UDP: {
int i, opened;

  	/* Only close socket when last channel is closed */
  	opened = 0;
  	for ( i=0; i<NET_MAX_CHANNEL; ++i ) {
  		if ( channels[i].active &&
  			(channels[i].id == channel->id) ) {
  			++opened;
  		}
  	}
  	if ( opened == 1 ) {
  		closesocket(channel->id);
  	}
  }
  break;
  case PROTO_TCP: {
  	closesocket(channel->id);
  }
  break;

}
channel->active = 0;
}

/* Write a 16/32 bit value to network packet buffer */
void Net_Write16(Uint16 value, Uint8 *area)
{
Uint16 *area16 = (Uint16 *)area;

*area16 = SDL_SwapBE16(value);
}
void Net_Write32(Uint32 value, Uint8 *area);
{
Uint32 *area32 = (Uint32 *)area;

*area32 = SDL_SwapBE32(value);
}

/* Read a 16/32 bit value from network packet buffer */
Uint16 Net_Read16(Uint8 *area);
{
Uint16 *area16 = (Uint16 *)area;

return(SDL_SwapBE16(*area16));
}
Uint32 Net_Read32(Uint8 *area);
{
Uint32 *area32 = (Uint32 *)area;

return(SDL_SwapBE32(*area32));
}

I’m thinking about redoing my read() to deal with vectors of packet buffers
to reduce overhead. It’s not completely clear it’s worth it, but if
there’s any overhead or setup associated with invoking sdlnet_read(),
the vector approach helps reduce it.

You should plan on people instantiating several sdlnet_ objects
(you are writing this as an object with no static variables, right?)
in the same program for various reasons, including creating
test apps.

  • Dan> -----Original Message-----

From: Sam Lantinga [SMTP:slouken at devolution.com]
Sent: Monday, October 26, 1998 11:53 AM
To: Kegel, Dan
Subject: Re: Can you comment on my implementation?

  1. why do you queue incoming udp messages? Just pass 'em straight back.
    And don’t let 'em tell you what channel to listen to - you tell them
    where
    the packet came from!

Wow, that’s much more useful than what I was doing, thanks! :slight_smile:

  1. might want to offer a vector interface - user gives
    you an array of sdlnet_pktbuf_t’s, you fill 'em.

Where is that useful?

  1. mixing tcp and udp IMHO is a bad idea. It makes it hard to support
    NAT’s.
    But I agree TCP has a lot of advantages for some stuff. e.g.
    if you implement your own reliable transport over UDP, you’re
    responsible for handling a lot of stuff just right.

Yep. It might occasionally be useful to crank out a background URL
retrieval in the middle of your game, at some point.

  1. your integer portability routines are not the prettiest. Gotta have
    ’em,
    though.
    I might be tempted to move them out into a separate module.

Good idea.

  1. you should provide a way to get a vector of all the network file
    descriptors
    so users can call select.

Not bad. Unfortunately, this isn’t portable to TLI, and the odd rare
hacked implementation of Berkeley sockets. I still might do it though,
since it would be REALLY useful on systems that support it.

FILE I/O sucks when you’re talking about portability.

Why doesn’t everyone just implement read() and write() and select() ?

More later, perhaps.

Thank you very much for your suggestions. Good stuff, simplifies my code
considerably. :slight_smile:

See ya!
-Sam Lantinga (slouken at devolution.com)


Author of Simple DirectMedia Layer -
http://www.devolution.com/~slouken/SDL/

IMHO the TCP stuff should be totally separate, so someone
who wants to implement a reliable protocol on top of UDP
can ignore the TCP stuff, and so you can optimize the TCP
interface for accessing web servers or whatever, without
bending over backwards to let games offer TCP services
(which won’t work well thru firewalls or NATs anyway).
But what do I know…

  • Dan> -----Original Message-----

From: Sam Lantinga [SMTP:slouken at devolution.com]
Sent: Monday, October 26, 1998 12:15 PM
To: Kegel, Dan
Subject: Re: Can you comment on my implementation?

I’m thinking about redoing my read() to deal with vectors of packet
buffers
to reduce overhead. It’s not completely clear it’s worth it, but if
there’s any overhead or setup associated with invoking sdlnet_read(),
the vector approach helps reduce it.

The vector approach is much more useful for UDP networking than TCP
networking, and it makes much more sense not to mix TCP and UDP if
you do it that way since there is no concept of a "listening socket"
in UDP.

Hum.
-Sam Lantinga (slouken at devolution.com)


Author of Simple DirectMedia Layer -
http://www.devolution.com/~slouken/SDL/