What's wrong with this server code ?? Cant read incoming data

[Jens]

Hello,

I have been looking for some C-code which listens on a user-defined port
for incoming data traffic. When data is received, the data is written to a
file.

I found some C-code (server) that almost does the job. It listens on a
user-defined
port and responds to incoming data by writing how many times somebody
has tried to connect to the server.

I modified the code, but the read function returns an error (-1) and
incoming data
is not written to the buffer.

Anybody out there who can tell me what I am doing wrong?

Here is the code:

/* server.c - code for example server program that uses TCP */
#ifndef unix
#define WIN32
#include <windows.h>
#include <winsock.h>
#else
#define closesocket close
#include <sys/types.h>
#include <sys/socket.h>
#include <netinet/in.h>
#include <netdb.h>
#endif

#include <stdio.h>
#include <string.h>

#define PROTOPORT 5193 /* default protocol port number */
#define QLEN 6 /* size of request queue */

int visits = 0; /* counts client connections */
int nbytes;
/*------------------------------------------------------------------------
* Program: server
*
* Purpose: allocate a socket and then repeatedly execute the following:
* (1) wait for the next connection from a client
* (2) send a short message to the client
* (3) close the connection
* (4) go back to step (1)
*
* Syntax: server [ port ]
*
* port - protocol port number to use
*
* Note: The port argument is optional. If no port is specified,
* the server uses the default given by PROTOPORT.
*
*------------------------------------------------------------------------
*/
main(argc, argv)
int argc;
char *argv[];
{
struct hostent *ptrh; /* pointer to a host table entry */
struct protoent *ptrp; /* pointer to a protocol table entry */
struct sockaddr_in sad; /* structure to hold server's address */
struct sockaddr_in cad; /* structure to hold client's address */
int sd, sd2; /* socket descriptors */
int port; /* protocol port number */
int alen; /* length of address */
char buf[1000]; /* buffer for string the server sends */

#ifdef WIN32
WSADATA wsaData;
WSAStartup(0x0101, &wsaData);
#endif
memset((char *)&sad,0,sizeof(sad)); /* clear sockaddr structure */
sad.sin_family = AF_INET; /* set family to Internet */
sad.sin_addr.s_addr = INADDR_ANY; /* set the local IP address */

/* Check command-line argument for protocol port and extract */
/* port number if one is specified. Otherwise, use the default */
/* port value given by constant PROTOPORT */

if (argc 1) { /* if argument specified */
port = atoi(argv[1]); /* convert argument to binary */
} else {
port = PROTOPORT; /* use default port number */
}
if (port 0) /* test for illegal value */
sad.sin_port = htons((u_short)port);
else { /* print error message and exit */
fprintf(stderr,"bad port number %s\n",argv[1]);
exit(1);
}

/* Map TCP transport protocol name to protocol number */

if ( ((int)(ptrp = getprotobyname("tcp"))) == 0) {
fprintf(stderr, "cannot map \"tcp\" to protocol number");
exit(1);
}

/* Create a socket */

sd = socket(PF_INET, SOCK_STREAM, ptrp->p_proto);
if (sd < 0) {
fprintf(stderr, "socket creation failed\n");
exit(1);
}

/* Bind a local address to the socket */

if (bind(sd, (struct sockaddr *)&sad, sizeof(sad)) < 0) {
fprintf(stderr,"bind failed\n");
exit(1);
}

/* Specify size of request queue */

if (listen(sd, QLEN) < 0) {
fprintf(stderr,"listen failed\n");
exit(1);
}

/* Main server loop - accept and handle requests */

while (1) {
alen = sizeof(cad);
if ( (sd2=accept(sd, (struct sockaddr *)&cad, &alen)) < 0) {
fprintf(stderr, "accept failed\n");
exit(1);
}
visits++;
//sprintf(buf,"This server has been contacted %d
time%s\n",visits,visits==1?".":"s.");
//send(sd2,buf,strlen(buf),0);

// read returns an error (-1)
nbytes=read(sd2,buf,strlen(buf));
printf("read %d bytes of data",nbytes);
closesocket(sd2);
}
}

[Ian Collins]

Jens wrote:

Hello,

I have been looking for some C-code which listens on a user-defined port
for incoming data traffic. When data is received, the data is written to a
file.

While sockets are OT here, one obvious error isn't:

char buf[1000]; /* buffer for string the server sends */

Note the array buf isn't initialised.

nbytes=read(sd2,buf,strlen(buf));

You are calling strlen on an initialised buffer.

Use a constant for the size.

--
Ian Collins.

[Ian Collins]

Ian Collins wrote:

Jens wrote:

>>Hello,

I have been looking for some C-code which listens on a user-defined port
for incoming data traffic. When data is received, the data is written to a
file.

While sockets are OT here, one obvious error isn't:

> char buf[1000]; /* buffer for string the server sends */

Note the array buf isn't initialised.

>>nbytes=read(sd2,buf,strlen(buf));

You are calling strlen on an initialised buffer.

Use a constant for the size.

Or sizeof(buf).

--
Ian Collins.

[Keith Thompson]

"Jens" <je**@jensen.notwrites:

I have been looking for some C-code which listens on a user-defined port
for incoming data traffic. When data is received, the data is written to a
file.

[snip]

You also posted this in comp.std.c. It's off-topic in both
newsgroups. See my respones there.

--
Keith Thompson (The_Other_Keith) ks***@mib.org <http://www.ghoti.net/~kst>
San Diego Supercomputer Center <* <http://users.sdsc.edu/~kst>
We must do something. This is something. Therefore, we must do this.

[Keith Thompson]

Flash Gordon <sp**@flash-gordon.me.ukwrites:

Jens wrote, On 23/01/07 21:56:

[...]

if (port 0) /* test for illegal value */

What about a number above 63353? (he says making use of off topic knowledge)

Do you mean 65535 (2**16-1)?

[...]

exit(1);

Non-portable return value. Include stdlib.h at the top then use
exit(EXIT_FAILURE);

Non-portable return values are not unreasonable in non-portable code.
Judging by the non-standard (or other-standard) library routines being
used, it's likely that "exit(1)" is a correct way to indicate an
error.

On the other hand, using "exit(EXIT_FAILURE)" on such systems is
likely to yield identical results.

If the program defines (and documents!) multiple failure codes, and 1
is just one among several such codes, then using exit(1) is reasonable
(though defining and using constants would be better). If you're
just signally generic failure, exit(EXIT_FAILURE) is probably
preferred.

[...]

--
Keith Thompson (The_Other_Keith) ks***@mib.org <http://www.ghoti.net/~kst>
San Diego Supercomputer Center <* <http://users.sdsc.edu/~kst>
We must do something. This is something. Therefore, we must do this.

[Flash Gordon]

Keith Thompson wrote, On 24/01/07 00:17:

Flash Gordon <sp**@flash-gordon.me.ukwrites:

>Jens wrote, On 23/01/07 21:56:

[...]

>> if (port 0) /* test for illegal value */

What about a number above 63353? (he says making use of off topic knowledge)

Do you mean 65535 (2**16-1)?

Yes. Just a tripeographical error. Although I can't see how I managed it!

[...]

>> exit(1);

Non-portable return value. Include stdlib.h at the top then use
exit(EXIT_FAILURE);

Non-portable return values are not unreasonable in non-portable code.
Judging by the non-standard (or other-standard) library routines being
used, it's likely that "exit(1)" is a correct way to indicate an
error.

On the other hand, using "exit(EXIT_FAILURE)" on such systems is
likely to yield identical results.

If the program defines (and documents!) multiple failure codes, and 1
is just one among several such codes, then using exit(1) is reasonable
(though defining and using constants would be better). If you're
just signally generic failure, exit(EXIT_FAILURE) is probably
preferred.

The only failure code used was 1. Had there been different codes for
different failures then, as you say, it would have been more reasonable.

So I basically agree with all your comments.
--
Flash Gordon