sockets problem

Code:

#include <stdio.h>
#include <stdlib.h>
#include <sys/socket.h>
#include <sys/types.h>
#include <sys/time.h>
#include <netinet/in.h>
#include <arpa/inet.h>
#include <string.h>
#include <unistd.h>

#define PORT 9017 
#define BUFFER  1024
#define MAXUSERS 10


int main(int argc, char* argv[])
{

    int sockfd, newfd;
    struct sockaddr_in myaddr; 
    struct sockaddr_in theiraddr;

    int port = -1;
    
    char* buffer = malloc(BUFFER);
    
    int addrlen;
    int maxfd;
    
    if (argc > 1)
    {
	int i;
	for (i = 0; i < 2; i++)
	{
	    if (strncmp(argv[i], "-p", 2) == 0)
	        port = atoi(argv[i + 1]);
	    if (strncmp(argv[i], "-h", 2) == 0)
	    {
		fprintf(stderr, "\nUsage: echoserv -p <port> -h\n");
		fprintf(stderr, "-p <port> sets the port that host server\n");
		fprintf(stderr, "-h prints this help screen\n\n");
		exit(EXIT_FAILURE);
	    }
	}
    }
    if (port == -1)
	port = PORT;
    
    printf("\n***ECHOSERV v0.0.2***\n");
printf("***Port is: %d***\n\n", port);
    
    fd_set master;
    fd_set slave;
    
    sockfd = socket(AF_INET, SOCK_STREAM, 0);
    
    myaddr.sin_family = AF_INET;
    myaddr.sin_port = htons(port);
    myaddr.sin_addr.s_addr = INADDR_ANY;
    
    bind(sockfd, (struct sockaddr *)&myaddr, sizeof(struct sockaddr));
    
    listen(sockfd, MAXUSERS);
    
    FD_ZERO(&master);
    FD_ZERO(&slave);
    
    FD_SET(sockfd, &master);
    maxfd = sockfd;
    
    int i;
    int data;
    
    for( ; ; ) 
    {
	slave = master;
	select(maxfd + 1, &slave, NULL, NULL, NULL);
	for (i = 0; i <= maxfd; i++)
	{
	    if (FD_ISSET(i, &slave)) /
	    {
		if (i == sockfd)
		{
		    addrlen = sizeof(theiraddr);
		    newfd = accept (sockfd, (struct sockaddr *)&theiraddr, &addrlen);
		}
		else
		{
		    FD_SET(newfd, &master); 
	    
	    	    if (newfd > maxfd) 
			maxfd = newfd;
		
		}
	    }
	    else
	    {
		data = recv(i, buffer, sizeof(buffer), 0);
		if (data <= 0) //disconnection or error
		{
		    printf("not good here!\n");
		    FD_CLR(i, &master); 		    close(i);
		}
		else
		{
		    send(i, buffer, sizeof(buffer), 0); 
		}
	    }
	}
    }
    close(newfd);
    close(sockfd);
    free(buffer);
    return 0;
}

hi everyone.

this code (an echo server) is giving me some problems. "not good here" gets printed the first time a connection is received and no data gets send()'t . i have worked with sockets before but not with select().

thx for help

Originally posted by Salem
> port = atoi(argv[i + 1]);
Ok, so what happens when the user types in

echoserv -p
or even
echoserv -p -1

well a segmentation fault does : )
thats nice but its not the cause of my problem.

> exit(EXIT_FAILURE);
Asking for help isn't normally a fatal error

understood.

> bind(sockfd, (struct sockaddr *)&myaddr, sizeof(struct sockaddr));
> listen(sockfd, MAXUSERS);
No checking of status results here.

laziness. they're fine though, obviously since i could telnet on the port.

> int i;
> int data;
Slipping into being C++ here

what, you can't declare integers in C? since when?

> select(maxfd + 1, &slave, NULL, NULL, NULL);
More ignored status results. You don't know whether select returned because of a timeout, or because there was some activity to report.
Since you don't provide a timeout, its going to be instant.

you mean something along the lines of checking wheter a 0 is returned (meaning a timeout) or a -1 error of some sort.

> data = recv(i, buffer, sizeof(buffer), 0);
buffer is a pointer, not an array
All you get here is the sizeof the pointer, not the size of the memory it points to.

Compare with
char buffer[BUFFER];

how about changing sizeof(buffer) with BUFFER.

> send(i, buffer, sizeof(buffer), 0);
At least it's compatible with your recv() error - receiving and sending in 4-byte blocks.
But it should be
send(i, buffer, data, 0); // the amount received.

why data? if i use BUFFER instead maybe for recv would i use BUFFER here as well?

Oh, and also check the return result from send() as well.

can do. you mean just a normal perror() on -1

> close(newfd);
How do you get out of your infinite for(; loop?

ok so ill just leave the fd's un close() 'd and the memory un free() 'd.

checklist:
[ ] Fix command line argument handling
[ ] Remove fatal error on help
[ ] Check status of bind(), listen(), and every send() and recv()
[ ] Be confused about slipping into C++
[ ] Check different statuses of select()
[ ] Fix handling of amounts of data
[ ] remove close() and free()

ill post my code later and tell you what happens.

thx for your help.

>You can only declare variables at the beginning of a block.
I can declare them outside a function as well, without any block, the global variables, can't I?

heres the new code:

Code:

#include <stdio.h>
#include <stdlib.h>
#include <sys/socket.h>
#include <sys/types.h>
#include <sys/time.h>
#include <netinet/in.h>
#include <arpa/inet.h>
#include <string.h>
#include <unistd.h>
#include <errno.h>

#define PORT 9017 
//this is the default port that will be used if program is run w/o the -p switch
#define BUFFER  1024
//how much data can be held in a single buffer
#define MAXUSERS 10
//basically, the backlog for the listen() function

int main(int argc, char* argv[])
{
    /* Variables used throughout the program */
    int sockfd, newfd; // file descriptors for the server and incoming connection
    
    struct sockaddr_in myaddr; // sockets for the connections
    struct sockaddr_in theiraddr;

    int port = -1;
    
    int i;
    int data;
    int nfound;
    
    char* buffer = malloc(BUFFER);
    
    int addrlen;
    int maxfd;
    
    if (argc > 1)
    {
	int i;
	for (i = 0; i < 2; i++)
	{
	    if (strncmp(argv[i], "-p", 2) == 0)
	    {
		if (argv[i + 1])
		{
		    if (atoi(argv[i + 1]) > 0)
		    {
	    		port = atoi(argv[i + 1]);
		    }
		    if (atoi(argv[i + 1]) <= 0)
		    {
			port = atoi(argv[i + 1]) * -1;
		    }
		}
		else
		{
		    fprintf(stderr, "\nPort not provided!");
		    fprintf(stderr, "\nUsage: echoserv -p <port> -h\n");
		    fprintf(stderr, "-p <port> sets the port that host server\n");
		    fprintf(stderr, "-h prints this help screen\n\n");
		    exit(EXIT_FAILURE);
		}
	    }
	    if (strncmp(argv[i], "-h", 2) == 0)
	    {
		fprintf(stderr, "\nUsage: echoserv -p <port> -h\n");
		fprintf(stderr, "-p <port> sets the port that host server\n");
		fprintf(stderr, "-h prints this help screen\n\n");
		exit(EXIT_SUCCESS);
	    }
	}
    }if (port == -1)
	port = PORT;
    
    printf("\n***ECHOSERV v0.0.2***\n");
    printf("***Port is: %d***\n", port);
    
    fd_set master;
    fd_set slave;
    
    if ((sockfd = socket(AF_INET, SOCK_STREAM, 0)) == -1)
    {
	perror("socket");
	exit(EXIT_FAILURE);
    }
    
    myaddr.sin_family = AF_INET;
    myaddr.sin_port = htons(port);
    myaddr.sin_addr.s_addr = INADDR_ANY;
    
    if ((bind(sockfd, (struct sockaddr *)&myaddr, sizeof(struct sockaddr)) == -1))
    {
	perror("bind");
	exit(EXIT_FAILURE);
    }
    
    if ((listen(sockfd, MAXUSERS)) == -1)
    {
	perror("listen");
	exit(EXIT_FAILURE);
    }
    
    FD_ZERO(&master);
    FD_ZERO(&slave);
    
    FD_SET(sockfd, &master);
    maxfd = sockfd;  
    for( ; ; ) /* forever and ever and ever and ever... */
    {
	slave = master;
	if ((nfound = select(maxfd + 1, &slave, NULL, NULL, NULL)) < 0)
	{
	    perror("select");
	    exit(EXIT_FAILURE);
	}
	
	for (i = 0; i <= maxfd; i++)
	{
	    if (FD_ISSET(i, &slave)) //if there are connections
	    {
		if (i == sockfd)
		{
		    addrlen = sizeof(theiraddr);
		    if ((newfd = accept (sockfd, (struct sockaddr *)&theiraddr, &addrlen)) == -1)
		    {
			perror("accept");
		    }
		}
		else
		{
		    FD_SET(newfd, &master); //place all new connections in master set
	    
	    	    if (newfd > maxfd) //update the maxfd variable
			maxfd = newfd;
		    FD_CLR(sockfd, &slave);
		
		}
	    }
	    else
	    {
		if ((data = recv(i, buffer, BUFFER , 0)) == -1)
		{
		    perror("recv");
		}
		if (data <= 0) //disconnection or error
		{
		    FD_CLR(i, &master); // cya later dude!
		    close(i);
		}
		else
		{
		    if ((send(i, buffer, data , 0)) == -1)
		    {
			perror("send");
		    } //copycat
		}
	    }
	}
    }
    return 0;
}

heres what happens when i run:
echoserv -p 9028

i telnet onto port 9028. perror prints
recv: socket operation on non-socket
three times and then if i open another telnet connection no more stuff is printed with perror.

im a bit confused about this.

thx for help

thanx salem i got it.