Chat program problem

[tal0n]

I am getting a run time error when I run a program. I will include the source to the server and client.

select: Bad File Number


// Server of tal0n

#include "tal0n_.h"
#include "list.h"
#define port 9090
#define BACKLOG 15

int main()
{
fd_set master; // master list for select()
struct sockaddr_in server; // server structure
struct sockaddr_in client;
struct timeval time; // client structure
int fdlarge; // hold largest file discriptor for select()
int top, newfd; // server socket file discriptor
char *buf; // buffer for clients
int addrlen, i, j;
WSADATA wsaData; // Needed for Windows Socket
char *announ, *user;
char *timeout = "Server timedout due to inactivity";
tal0n_ tserv;
int bytes;

time.tv_sec = 120;

if ( WSAStartup(MAKEWORD(1,1), &wsaData) != 0 )
{
cout << "Error in WSAStartup";
return 1;
}

FD_ZERO(&master);

if (( top = socket( AF_INET, SOCK_STREAM, 0)) == -1)
{
cout << "Error in socket";
return 1;
}

server.sin_family = AF_INET;
server.sin_addr.s_addr = INADDR_ANY;
server.sin_port = htons(port);
memset( &(server.sin_zero), '\0', 8);

if ( bind(top, ( struct sockaddr *)&server, sizeof(server)) == -1)
{
cout << "Error in bind";
return 1;
}

if ( listen( top, BACKLOG) == -1)
{
cout << "Error in listen";
return 1;
}

FD_SET(top, &master);

fdlarge = top;

while (1)
{
if( select( fdlarge+1, &master, NULL, NULL, &time) == -1)
{
cout << "Select Error";
return 1;
}

for ( int i=0; i <= fdlarge; i++ )
{
if ( FD_ISSET(i, &master) )
{
if (i == top)
{
// Handle new connections
int size = sizeof(client);
if ((newfd = accept(top, (struct sockaddr *)&client, &size)) == -1)
cout << "Error in Accept";
else
{
FD_SET(newfd, &master); // Add to set
if (newfd > fdlarge)
fdlarge = newfd;
announ = newuser( i, &client );
tserv.announce(announ, fdlarge, top);

// Show Status
cout << "selectserver: new connection from " << inet_ntoa(client.sin_addr) << endl;
}
}
else
{
if ( (bytes = recv( i, buf, sizeof(buf), 0)) < 0 )
{
cout << i << " disconnected";
closesocket(i);
FD_CLR(i, &master);
return 1;
}
else
{
user = getuser( tserv.getIp( i ) );
for ( int j=0; j <= fdlarge; j++ )
{
if ( j != top )
{
user = strcat( user, ": " );
user = strcat( user, buf );
tserv.sendall( j, user, bytes );
}
}
}
}
}
else
{
for ( int l=0; l <= fdlarge; l++ )
{
if ( l != top )
{
tserv.sendall( j, timeout, strlen(timeout) );
FD_CLR( l, &master );
}
}
}
}
}
return 0;
}


//Client tal0n
#include "tal0n_.h"
#define PORT 9090
#define MAX 256

struct sockaddr_in them;
struct hostent *he;
fd_set readfds, writefds;

int main(int argc, char *argv[])
{
tal0n_ cl;
char *prompt=">>";
char buf[MAX], rec[MAX];
char name[MAX];
int unsigned top, fdlarge, len;
WSADATA info;
FD_ZERO(&writefds);
FD_ZERO(&readfds);

FD_SET(top, &readfds);
FD_SET(top, &writefds);

if ( argc < 2 )
{
cout << "Usage: tal0n <IP address or Host Name>";
return 1;
}

cout << "Enter your user name: ";
cin >> name;

if (WSAStartup(MAKEWORD(1, 1), &info) != 0)
{
perror("WSAStartup");
return 1;
}

if((he=gethostbyname(argv[1])) == NULL )
{
perror("gethostbyname");
return 1;
}

if (( top=socket(AF_INET, SOCK_STREAM, 0)) == -1 )
{
perror("socket");
return 1;
}

fdlarge = top;

them.sin_family=AF_INET;
them.sin_port=htons(PORT);
them.sin_addr=*((struct in_addr *)he->h_addr);
memset( &(them.sin_zero), '\0', 8); // Zero out rest of the structure

if (connect(top, (struct sockaddr *)&them, sizeof(struct sockaddr)) == -1)
{
perror("connect");
return 0;
}

if ( cl.sendall( top, name, sizeof(name)) == -1 )
{
perror("sendall");
return 1;
}

//Main Loop
while(1)
{
if (select( fdlarge+1, &readfds, &writefds, NULL, NULL) < 0)
{
perror("select");
return 1;
}

if (FD_ISSET(top, &readfds))
{
if (recv(top, rec, sizeof(buf), 0) <= 0)
{
cout << "Server disconnected." << endl;
return 1;
}

rec[strlen(rec) + 1] = '\0';
cout << rec << endl;
}

cout << prompt;
cin >> buf;
len = strlen(buf) - 1;
if (cl.quitme( buf, len ) == 1 )
{
buf[len-3] = '\0';
if ( cl.sendall( top, name, sizeof(name)) == -1 )
{
perror("sendall");
return 1;
}
closesocket(top);
cout << "Goodbye";
}
else
{
if ( cl.sendall( top, name, sizeof(name)) == -1 )
{
perror("sendall");
return 1;
}
}
}
closesocket(top);
return 0;
}

[RobSeace]

At a quick glance, I see several issues with both bits of code...
I assume the one giving the "bad file number" error is the client?
Because, in that one you do two FD_SET()'s at the very beginning,
referencing a stack variable ("top"), which you haven't even set
to any value yet! So, you're setting some bogus FD#, based on
whatever random junk value that happens to be in "top", at that
point... You need to move those after you assign a value to "top"
(ie: after the socket() call)...

But, also in both client and server, you appear to not be using
separate temp fd_sets for your select() call... You must do that,
because select() modifies the fd_sets you pass to it (otherwise,
you couldn't check them to see what FDs are still set)... You
need to maintain one outside master copy of your fd_sets, then
temp copies which you initialize with the contents of the masters
prior to every select() call...

[tal0n]

I made the changes and still the same error but I got less warnings at compile time.

[tal0n]

I fixed the compiler errors, and now I recieve run time errors. It is in the client where after the user picks his name and attempts to chat. It will send one message but segmentation fault on the second message sent.

//-----CUT-----//
cout << prompt << " ";
cin.getline( buf, LENGTH, '\n');
len = strlen(buf);
if ( len > LENGTH ) // Prevent the user from entering a message that is to big
cout << "The string is to long" << endl;
else
{
if (cl.quitme( buf, len ) == 1 )
{
buf[len-3] = '\0';
if ( cl.sendall( top, buf, sizeof(name)) == -1 )
{
cout << "sendall";
return 1;
}
closesocket(top);
cout << "Goodbye";
}
else
{
if ( cl.sendall( top, buf, sizeof(name)) == -1 )
{
cout << "sendall";
return 1;
}
}
}
}
closesocket(top);
return 0;
//-----CUT-----//


LENGTH is set to 1500, rec is a char*, buf is a char*, top is the socket file descriptor.

[tal0n]

ohh and prompt is a simple ">>" string.

[Loco]

Are you still using the following "buf" definition?

#define MAX 256
...
int main(int argc, char *argv[])
{
tal0n_ cl;
char *prompt=">>";
char buf[MAX], rec[MAX];
...
It could be caused by a buffer overrun (the buf variable is too small to hold LENGTH characters.

However, I don't think the problem lies in this part of the code (to be honest I didn't read too much of it), but in the cl.sendall() function or the cl.quitme() function... I promise I'll take a closer look at the code to help you find out what might be causing the problem!!!!

zzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzZZZZZZZZZZZZZZZZZ ZZZ
8O

[tal0n]

Well then you may want the sendall and quiteme functions:

int tal0n_::sendall(int sockfd, char *buf, int len)
{
int total = 0; // how many bytes we've sent
int bytesleft = len; // how many we have left to send
int n;

while(total < len)
{
n = send(sockfd, buf+total, bytesleft, 0);
if (n == -1)
break;
total += n;
bytesleft -= n;
}

return (n==-1)?-1:0; // return -1 on failure, 0 on success
}

int tal0n_::quitme( char *s, int len)
{
char *p;
int test, pos;
for ( int i=0; i < len; i++ )
{
p = strchr( s, '/');
pos = *p;
test = (s[pos+1]=='q') ? 1 : 0 ;
}
return (test==1)?1:0;
}

[RobSeace]

Well, that quitme() function sure looks suspicious, alright...
First off, you're not checking if strchr() fails... If there's no "/"
at all in the buffer, you'll dereference a NULL pointer, and die...
Secondly, you seem to be misusing "*p" as if it were the index
into "s" of where "p" is located... In this case, "*p" is always
going to be '/', aka 47 in decimal... So, you're always then
indexing to "s[48]" to check for 'q'... I think you just want
something like "test = (*(p+1) == 'q');", assuming you want
to be looking for 'q' immediately following the '/'... Also, what
exactly is that for() loop accomplishing???? Nowhere are you
changing the value of "s", so every single time through the loop,
you'd be doing the same exact comparison...