Re: client-server network

Tech-Archive recommends: Repair Windows Errors & Optimize Windows Performance

See below...
On 15 Dec 2006 03:11:56 -0800, "meenu" <mnair.lekshmi@xxxxxxxxx> wrote:


meenu wrote:
I have created a server and a client to implement a client-server
network using UDP (technique of broadcast) ......client will first send
the msg using sendto..then waits for a reply from the server using
recvfrom...server received the data that the client sent.But the data
sent by server didn't reach the client......

Please find the attached files............

ServerUDP.c


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


void Broadcast()
{
SOCKET listenSocket;
SOCKET remoteSocket;
SOCKADDR_IN saServer;

char szBuf[256] = "";
char RecszBuf[256] = "";

int nRet = 0;
int nLen = 0;
int err=0;
int Bound=0;

WORD wVersionRequested = MAKEWORD(1,1);
WSADATA wsaData;


short nPort= 2000;


/* Initialize WinSock and check version */
nRet = WSAStartup(wVersionRequested, &wsaData);

if (wsaData.wVersion != wVersionRequested)
{
fprintf (stderr,"\n Wrong version\n");
return;
}


/* Wait for an incoming request */
listenSocket = socket ( AF_INET, /* Address family */
SOCK_DGRAM, /* Socket type */
IPPROTO_UDP ); /* Protocol */

if (INVALID_SOCKET == listenSocket)
{
printf("error");
****
You will find it a lot easier to debug if instead of printing the same message in each
place you were to do
printf("listen socket creation failure %d\n", WSAGetLastError());
****

return;
}


/* Fill in the address structure */
saServer.sin_family = AF_INET;
saServer.sin_addr.s_addr = INADDR_ANY; /* Let WinSock supply address
*/
saServer.sin_port = htons(2000);
****
BOOL opt = TRUE;
SetSockOpt(listensocket, SOL_SOCKET, SO_REUSEADDR, (LPCSTR)&opt, sizeof(opt));
*****

/* bind the name to the socket */
Bound = bind ( listenSocket, /* Socket */
(SOCKADDR *)&saServer, /* Our address */
sizeof(struct sockaddr) ); /* Size of address structure */

if (SOCKET_ERROR == nRet)
{
printf("error");

return;
}

nLen = sizeof(SOCKADDR);
nRet = gethostname (szBuf, sizeof(szBuf));

if (SOCKET_ERROR == nRet)
{
printf("error");
return;
}

printf("\n\nServer named %s waiting on port 2000\n",szBuf);
*****
LOOP BOUNDARY (see below)
while(TRUE)
{ /* server loop */
*****

memset (szBuf, 0, sizeof(szBuf));
err=sizeof(saServer);

nRet = recvfrom( listenSocket, /* Connected client */
RecszBuf, /* Receive buffer */
sizeof(RecszBuf), /* Length of buffer */
0 ,(SOCKADDR *)&saServer,&err); /* Flags */
****
Why are you reusing the saServer structure to hold client information? Either change the
name so it does not include the word 'Server' anywhere, or the word 'Client', or use a
different structure entirely such as saClient, so there is no confusion over what is going
on.

The memset and this code are both wrong. There is no reason to zero out a buffer you are
about to overwrite. Of course, you could make the argument that no matter how may bytes
you receive, you'll always have a NUL character at the end, but because you said
sizeof(RecszBuf) instead of sizeof(RecszBuff)-1, if you receive exactly as many bytes as
were sent, there will be no NUL at the end.

Note that the maximum packet length of UDP varies but is specified as being at least 536
bytes, which, when packet overheads are added, leaves 512 bytes for the message. You
allocated 256 byte buffers, so the possibility that you WILL in fact fill your buffer all
the way to the end is significant. Buffer-overflow exploits happen this way.

Forget the memset. Instead, (after making sure you can't read more than
sizeof(RecszBuf)-1 bytes) do
if(nRet >= 0)
RecszBuf[nRet] = '\0';

****

if (INVALID_SOCKET == nRet)
{
printf("error");
return;
}


printf("\nData received: %s", RecszBuf);
****
Perhaps you mean "\nData Received \"%s\""
Otherwise, you can't tell if the data is empty or cotains a lot of spaces exactly what was
received.

It is also good to adopt the convention that a line ENDS with a \n rather than the
convention that the next line BEGINS with a \n.
****

strcpy(szBuf,"Fromserver");
printf ("\nData to be send: %s \n\n\n", szBuf);

remoteSocket = accept( listenSocket, /* Listening socket */
NULL, /* Optional client address */
NULL );
*****
What is a TCP/IP 'accept' doing in the middle of a UDP server? You've already done the
recvfrom above! I note that you do not actually check the return result of this and print
the confusing message "error", but if you did, it would probably tell you that this call
was illegal.
****
Bound = bind ( listenSocket, /* Socket */
(SOCKADDR *)&saServer, /* Our address */
sizeof(struct sockaddr) ); /* Size of address structure */
****
What are you binding here? This doesn't even make sense. You've already bound your
server scoket, and this would be binding some other random socket value, apparently the
socket value used by the client, which doesn't make any sense whatsoeverto the server. You
only 'bind' a socket that you want visible to external users on the network, not the
random transient socket used by some random client. Otherwise, the next time some client
sends you a message on socket 52023 this will fail and you will get an error message.
****

if (SOCKET_ERROR == nRet)
{
printf("error");

return;
****
If you make this a server loop, it should be 'continue', not 'return'. If you like to use
'return', then you need to split this into two subroutines, one that does setup (creating
and binding the socket) and one of which does the receive/reply. Putting them both into a
function like this is creates all kinds of errors. For example, by returning, you have
not actually closed the socket, and it remains bound, so the next attempt to call this
will fail because the socket already exists!
}
****
Eliminate both the accept and bind. THey are wrong.
****
nRet = sendto ( listenSocket,
szBuf,
strlen (szBuf),
0 ,(SOCKADDR *)&saServer,sizeof(saServer));
*****
LOOP BOUNDARY (see below)
} /* server loop */

closesocket(listenSocket);
****
This is also wrong. You don't close the socket until you are done. You are not done. You
are going to go back to the caller and loop again. Wrong. The correct place to put the
loop is where I've marked LOOP BOUNDARY. Otherwise, there is SUBSTANTIAL overhead in
unloading, loading, initializing, binding, etc. and during that interval your server
simply does not exist. Since the cost of doing all of the setup is vastly higher than the
cost of the reply, your server will spend most of its time not existing (as far as the
client machine are concerned), which is not a good way to build a server.

THis also means that a lot of those errors need to be handled not by 'return' but
'continue', so the loop resumes. If there is a serious error, you handle it by 'break' so
that the socket is properly closed.

If you split this up so the loop function is in a separate subroutine from the setup, then
it should be a boolean function that returns true if the loop should continue and false if
the loop should terminate. You don't terminate the loop unless there is a server error; a
client-induced error just causes the message to be ignored, but the loop continues.
*****

return;
}


int main()
{
//do{
Broadcast();
//}while(1);
*****
If you have a loop it makes more sense to do
while(TRUE)
{
}

so that the condition of infinite looping is visually established before the rest of the
code is written. Only use do-while when the loop MUST be done at least once to establish
the condition for continuation.
****
}






ClientUDP.c


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



LPHOSTENT lpHostEntry;
SOCKET theSocket;
SOCKADDR_IN saServer;


void Broadcast()
{
int nRet=0;
int err=0;
char szBuf[256]="";
char szServer[256]="";
char RecszBuf[256]="";

WORD wVersionRequested = MAKEWORD(1,1);
WSADATA wsaData;

short nPort;
char str[250] = "";


nPort = 2000;

nRet = WSAStartup (wVersionRequested, &wsaData);

if (wsaData.wVersion != wVersionRequested)
{
fprintf (stderr,"\n Wrong version\n");
return;
}


/*Create a UDP stream socket */
theSocket = socket ( AF_INET, /* Address family */
SOCK_DGRAM, /* Socket type */
IPPROTO_UDP ); /* Protocol */

if (INVALID_SOCKET == theSocket)
{
printf("error");
return;
}

/* Fill in the address structure */
saServer.sin_family = AF_INET;
saServer.sin_addr.s_addr = inet_addr("10.1.13.42");
saServer.sin_port = htons(2000);


/* connect to the server */
nRet = connect ( theSocket, /* Socket */
( SOCKADDR *) &saServer, /* Server address */
sizeof(saServer)); /* Length of server address structure */

if (SOCKET_ERROR == nRet)
{
printf("error");
return;
}

strcpy(szBuf,"FromClient");
printf("send data : %s",szBuf);

err=sendto(theSocket, szBuf, sizeof(szBuf), 0 ,(SOCKADDR
*)&saServer,sizeof(saServer));
err=sizeof(saServer);
recvfrom(theSocket,RecszBuf,sizeof(RecszBuf),0,(SOCKADDR
*)&saServer,&err);

printf("\nrecvd data :%s",RecszBuf);

closesocket(theSocket);

return;
}


void main()
{

Broadcast();

}
Joseph M. Newcomer [MVP]
email: newcomer@xxxxxxxxxxxx
Web: http://www.flounder.com
MVP Tips: http://www.flounder.com/mvp_tips.htm
.

Relevant Pages

  • Re: multiple tcp server and client execution with close problem
    ... Ignore the loop for accept. ... for acceptafter settting socket to blocking mode. ... Client side: ... execute main to run server 3 times: ...
    (comp.unix.programmer)
  • Re: multiple tcp server and client execution with close problem
    ... while loop to accept ... Why is theserverstill waiting for the client? ... The server is still waiting because there is another function calling ...
    (comp.unix.programmer)
  • Re: Structure of the multitasking server
    ... is written the maximum number of server per Check_Selector is 27. ... GNAT.Sockets.Create_Selector uses two socket and the TCP/IP sub-system ... for I in reverse S'Range loop ... This is more or less what I came up with, where the "channel" is a ...
    (comp.lang.ada)
  • Re: multiple tcp server and client execution with close problem
    ... for acceptafter settting socket to blocking mode. ... I didn't understand what you meant by "while loop to ". ... Client side: ... execute main to run server 3 times: ...
    (comp.unix.programmer)
  • Re: Threading and returning values
    ... The e.Result is not coming out of the loop. ... I have now got my SMTP server apparently running correctly now, ... BackgroundWorker bw = new BackgroundWorker; ... // Note that in the Click event handler, ...
    (microsoft.public.dotnet.languages.csharp)